Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#9614 closed task (fixed)

avoid setting attributes on DomNode JS objects (IE leak problem)

Reported by: bill Owned by: bill
Priority: high Milestone: 1.4
Component: Dijit Version: 1.3.2
Keywords: Cc: liucougar, Tom Trenka
Blocked By: Blocking:

Description

Dijit has at least a few places where it sets attributes directly on the JS object associated with a DomNode:

Popup.js:

wrapper.dijitPopupParent=args.parent.id;

Menu.js:

node[this.id] = this._bindings.push([

Even though we are just setting string values, this causes problems on IE. To plagiarize Tom:

The issue is that when you start treating DOMNodes as if they were JS objects (i.e. hashtables), the JScript engine is forced to transparently wrap that node in an "expando" object, which (IIRC) won't ever be destroyed unless you specifically null the properties on the node that are not a native part of the node (or has script bindings).

Adding the value to the attributes array, on the other hand, shouldn't cause any major leaks (or at least force the expando wrapper to come into play), since an attribute is already a basic key/value pairing.

Change History (3)

comment:1 Changed 10 years ago by bill

(In [19117]) Avoid memory leak on IE by using dojo.attr() rather than setting a property on the node's JS object. Refs #9614.

comment:2 Changed 10 years ago by bill

Resolution: fixed
Status: newclosed

Menu fixed in [19118].

comment:3 Changed 10 years ago by bill

(In [19652]) Number of fixes (!strict) around Tree DnD:

  • When dragging from a Tree and dropping somewhere, source.getItem() should return a dojo.dnd.Item, not a DomNode? (fixes #7058)
    • Pass source to itemCreator() so that the function can see the type of the dragged nodes. Also fixed documentation of itemCreator(). (fixes #9683)
  • fix dragging a random (non TreeNode?) widget and dropping onto the tree. In this case Tree should not assume the dragged widget has a dojo.data item, parentNode, etc. like TreeNode?'s do. (fixes #9550)
  • better documentation (fixes #9684)
  • make Tree drag source support forInSelectedItems(), like dojo.dnd.Source (fixes #9685)
  • Avoid memory leak on IE by using dojo.attr(node, "id", ...) rather than setting id on the node's JS object (not sure if this matters though since id is a standard attribute name, not an expando property). (refs #9614)

I wasn't sure what fields are best to set in the object returned from getItem(). For now it's just setting type == treeNode and data == the TreeNode? widget.

Note: See TracTickets for help on using tickets.