Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#15216 closed task (fixed)

reduce/eliminate dependence on dojo/_base/window::doc "global"

Reported by: bill Owned by: bill
Priority: undecided Milestone: 1.8
Component: Dijit Version: 1.7.2
Keywords: Cc: Akira Sudoh
Blocked By: Blocking:

Description (last modified by bill)

There are lots of places in dijit that reference dojo/_base/window::doc (aka win.doc or dojo.doc) either directly, or indirectly, for example win.body(). But each widget essentially has a document that is set at construction time, usually implied by srcNodeRef.ownerDocument. So, widgets' references to win.doc typically create an unnecessary dependency on dojo/_base/window, and also preclude creating those widgets in another frame.

Most of the code in core doesn't have this problem. dom.byId(), for example, takes an optional document argument, and domConstruct.place() can infer the document via the refNode However, there are a few places in core that should be cleaned as it makes unnecessary assumptions about which document to use.

So, this ticket covers adding a this.doc member to _WidgetBase to point to the widget's document, removing unnecessary references to win.doc for dijit widgets, and cleaning up a few spots in core.

See spreadsheet from Akira for list of places to update.

Attachments (1)

selectionWindowParam.patch (43.7 KB) - added by bill 8 years ago.
patch against [28413] to add window param to selection API methods that don't have a DOMNode, saved for posterity

Download all attachments as: .zip

Change History (29)

comment:1 Changed 8 years ago by bill

Cc: Akira Sudoh added
Milestone: tbd1.8
Owner: set to bill
Status: newassigned
Type: defecttask

comment:2 Changed 8 years ago by bill

In [28401]:

Removing dependency on win.doc global-ish variable:

  • Each widget has a document determined at creation time, usually implied by srcNodeRef.ownerDocument but also specifiable explicitly. Save that in this.ownerDocument, and for convenience also record the associated <body> node in this.ownerDocumentBody. Maybe this.ownerDocument should be called this.doc instead, but I thought that might be confusing with Editor.document.
  • In widgets, use this.ownerDocument instead of win.doc, and this.ownerDocumentBody instead of win.body().
  • Since domConstruct.create() doesn't take a document parameter, either pass in a refNode (which implies the document), or avoid using domConstruct.create(). Could also have wrapped the domConstruct.create() calls with win.withDoc(), or used domConstruct.toDom().
  • Miscellaneous fixes to widgets.
  • Make dojo/_base/window::body() take an optional document parameter.

Editor not yet converted.

Refs #15216 !strict.

comment:3 Changed 8 years ago by bill

Description: modified (diff)

Filed:

  • #15525 for issues with lite engine.
  • #15226 for adding doc param to domConstruct.create()
  • #15229 for other changes to dojo core DOM methods
Last edited 8 years ago by bill (previous) (diff)

comment:4 Changed 8 years ago by bill

In [28402]:

Use target.ownerDocument rather than document, in case on.emit() is called on a node in a different document, refs #15216 !strict.

comment:5 Changed 8 years ago by bill

In [28403]:

Use winUtils.get() to get window from document. Refs #15216 !strict.

comment:6 Changed 8 years ago by bill

In [28406]:

Remove some apparently overcautious end-of-loop checking that happened to be referencing win.doc from possibly the wrong document.

Refs #15216 !strict.

comment:7 Changed 8 years ago by bill

In [28413]:

A few fixes around win.doc references:

  1. Standardize on this._sCall() interface to access selection API. Also looked at passing window parameter to each selection method, or creating a Selection class that's attached to a window on creation... but just did the simple thing for now. OTOH the range API takes a window parameter.
  1. remove unneeded win.withGlobal(), win.withDoc() calls
  1. misc cleanup

Refs #15216 !strict.

Changed 8 years ago by bill

Attachment: selectionWindowParam.patch added

patch against [28413] to add window param to selection API methods that don't have a DOMNode, saved for posterity

comment:8 Changed 8 years ago by bill

In [28434]:

No need for win.withGlobal() calls anymore, refs #15216 !strict.

comment:9 Changed 8 years ago by bill

In [28436]:

Remove some unused dependencies plus supply missing document parameter to dom.byId(), refs #15216 !strict.

comment:10 Changed 8 years ago by bill

In [28442]:

dojo/editor plugins: use this._sCall() interface to access selection API, rather than dojo.withGlobal().

Refs #15216

comment:11 Changed 8 years ago by bill

In [28443]:

dojo/editor plugins: remove remaining calls to dojo.withGlobal().

Refs #15216.

comment:12 Changed 8 years ago by bill

In [28444]:

missed a few spots, refs #15216 !strict

comment:13 Changed 8 years ago by bill

In [28446]:

Fix error from [28401], which removed the dojo/_base/window dependency but left references to it, thanks Sudoh-san, refs #15216 !strict.

comment:14 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

I'm going to close this and track any further changes in #15275, plus the tickets for individual modules #15525, #15226, and #15229. Probably should file a specific ticket for the dojo/DnD module too.

comment:15 Changed 8 years ago by bill

In [28472]:

Fix a few calls to this._sCall(), the last parameter needs to be an array, refs #15216 !strict.

comment:16 Changed 8 years ago by bill

In [28511]:

Fix missed conversions of selectionapi calls to this._sCall() in [28413], causing EnterKeyHandling test to fail on chrome, refs #15216 !strict.

comment:17 Changed 8 years ago by bill

In [28552]:

removed another unused ref to dojo/_base/window, refs #15216 !strict

comment:18 Changed 8 years ago by liucougar

In [28609]:

Refs #15216: dijit.range.create() argument is optional until 2.0

fix broken backward compatibility

comment:19 Changed 7 years ago by bill

In [28640]: fix typos from [28401], thanks Sudoh-san

comment:20 Changed 7 years ago by bill

In [28812]:

Even though DOMNode has an attribute called ownerDocument, _WidgetBase.ownerDocument should not be set on this.domNode (or this.focusNode). It's unnecessary and causes errors on IE.

Also adding in missing ownerDocument parameter when widgets create supporting widgets without specifying a srcNodeRef. (When srcNodeRef is specified, the ownerDocument is implied.)

Refs #15216 !strict. Thanks Sudoh-san for catching this.

comment:21 Changed 7 years ago by bill

In [29407]:

remove dependence on global document var, refs #15216 !strict

comment:22 Changed 7 years ago by bill

In [29408]:

remove dependence on global document var, refs #15216 !strict

comment:23 Changed 7 years ago by bill

In [30673]:

fix old comments that refer to dojo/_base/window::doc, even though it isn't being used anymore, refs #15216 !strict

comment:24 Changed 7 years ago by bill

In [30674]:

Replace global uses of deprecated dojo/_base/window::doc with "document", since win.doc always evaluates to document when there is no enclosing withDoc() or withGlobal() method. Refs #15216 !strict.

comment:25 Changed 7 years ago by bill

In [30694]:

Replace global use of deprecated dojo/_base/window::global with "window", since win.global always evaluates to window when there is no enclosing withGlobal() method. Refs #15216 !strict.

comment:26 Changed 7 years ago by bill

In [30695]:

Update comment to match changes in [30674], refs #15216 !strict.

comment:27 Changed 7 years ago by bill

In [30696]:

When converting stylesheet relative URL's to absolute URL's, use "location" from Editor.ownerDocument.parentWindow, rather than from dojo/_base/window::global. This is mainly for consistency with the other changes in #15216, and to avoid use of the likely-to-be-deprecated dojo/_base/window::global variable. I'm sure I could have also just used the global "window" variable also. Refs #15216 !strict.

comment:28 Changed 5 years ago by Bill Keese <bill@…>

In c535128334b5a130b78c79b87d2b236ab0490d9b/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.