Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2931 closed defect (fixed)

need destroyNode in 0.9

Reported by: Adam Peller Owned by: alex
Priority: high Milestone: 0.9beta
Component: HTML Version: 0.9
Keywords: leak 4dijit Cc: bill, sjmiles, alex, mike.schall@…
Blocked By: Blocking:

Description (last modified by bill)

removeChild leak protection needed in 0.9 dojo core or base. Refs #1727

let's just port the (simple) destroyNode code and not worry about deferred removal cases like removeNode.

then need fix all references in dijit to call destroyNode() rather than parentNode.removeChild() (look for PORT comments)

Change History (11)

comment:1 Changed 12 years ago by bill

Description: modified (diff)

comment:2 Changed 12 years ago by bill

(In [8743]) Prevent error about this.domNode not defined when hitting back button from Tree test page. Refs #2931.

comment:3 Changed 12 years ago by alex

Owner: changed from Adam Peller to alex
Status: newassigned

comment:4 Changed 12 years ago by bill

Component: DOMHTML

Talked to Alex about this; he thought using this.outerHTML is dangerous. But apparently there's a memory leak issue, right? The alternative clearing this.outerHTML is to put the node in a dummy "garbage can" bin and then clear that bin's innerHTML.

comment:5 Changed 12 years ago by guest

The garbage can came from the drip/sieve site. And that is how I proposed it in the .4 days. I think Cougar was having problems with this across iframes/documents. Each document would need to have it's own garbage can and clear it. The outerHTML was proposed at that time to workaround the multiple document issue.

comment:6 Changed 12 years ago by bill

Keywords: 4dijit added

Note: we talked with Alex about this about a month ago; we agreed he'd add a semi-public_destroyNode() method that we could call and then we would test it and see if it made things better/worse.

comment:7 Changed 12 years ago by alex

Resolution: fixed
Status: assignedclosed

(In [9407]) adding dojo._destroyElement method. Fixes #2931

comment:8 Changed 12 years ago by bill

(In [9414]) Update dijit to use dojo._destroyElement(). Refs #2931.

comment:9 Changed 12 years ago by bill

(In [9415]) Rename backgroundIframe.remove() to destroy() to match dojo naming conventions. Refs #2931.

comment:10 Changed 12 years ago by bill

(In [9420]) dojo._destroyElement argument doesn't necessarily have a parent node, specifically in the case of closing a tab (in a dijit.TabContainer?). Refs #2931.

comment:11 Changed 12 years ago by bill

(In [9421]) Using dojo._destroyNode() in this createNodesFromText() wasn't appropriate. Refactor code. Refs #2931.

Note: See TracTickets for help on using tickets.