Opened 10 years ago

Closed 8 years ago

#7976 closed defect (wontfix)

NodeList.empty() doesn't work in IE on certain nodes

Reported by: Eugene Lazutkin Owned by: Eugene Lazutkin
Priority: high Milestone: future
Component: Core Version: 1.2.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Eugene Lazutkin)

The NodeList defines a function that safely dispatches child nodes of listed parents. It doesn't work on a number of nodes in IE, because it uses innerHTML = "" trick resulting in exceptions for nodes with read-only innerHTML. See #7890 for details.

Additional problems:

  • It is not available for stand-alone nodes unless wrapped with NodeList.
  • Its body is very simple, yet involves an overhead of compiling a function on every call.

Proposal:

  1. Move this function to dojo/_base/html.js and make it available for stand-alone nodes too.
  2. Compile its body only once.
  3. Implement it differently for IE.

Possible implementation on IE:

dojo.empty = function(/* Node */ node){
  var c;
  while(c = node.lastChild){
    dojo._destroyElement(c);
  }
  return node; // Node
};

Change History (11)

comment:1 Changed 10 years ago by Eugene Lazutkin

Description: modified (diff)

See #7975 for notes for suggested dojo._destroyElement() improvements.

comment:2 Changed 10 years ago by Eugene Lazutkin

Milestone: tbdfuture
Owner: changed from anonymous to Eugene Lazutkin

Reassigning some core tickets to me as agreed with jburke.

comment:3 Changed 10 years ago by Eugene Lazutkin

(In [16227]) Implementing dojo.empty() with unit tests, !strict, refs #7976.

comment:4 Changed 10 years ago by Eugene Lazutkin

The next step is the refactoring of NodeList?. It will be tracked in #8282.

comment:5 Changed 10 years ago by Eugene Lazutkin

LiuCougar proposes the following algorithm for IE:

  1. Clone the node without children: var newNode = node.cloneNode(false);
  2. Replace the node: node.parentNode.replaceChild(newNode, node);
  3. Kill the node: dojo.destroy(node);

The relevant reference: http://blog.stevenlevithan.com/archives/faster-than-innerhtml --- according to it this method is faster than the "innerHTML" method for many things.

I definitely will include this method in the performance testing I plan to do later.

Additionally this method can be useful for dojo.destroy() implementation, and (according to the article) for innerHTML replacement on non-IE browsers.

comment:6 Changed 10 years ago by James Burke

Will using cloneNode(false) have an effect on any DOM event listeners on the cloned node?

comment:7 Changed 10 years ago by Eugene Lazutkin

Status: newassigned

Good point, we should test for it, but I suspect it will not honor event handlers. :-(

comment:8 Changed 10 years ago by Eugene Lazutkin

(In [16299]) Aliasing dojo.html._emptyNode() to dojo.empty(), !strict, refs #7976.

comment:9 Changed 10 years ago by Eugene Lazutkin

Resolution: duplicate
Status: assignedclosed

Closing this ticket in favor of #8282.

comment:10 Changed 9 years ago by pavelpenchev

Resolution: duplicate
Status: closedreopened

Using IE7 and dojo 1.5.0 + some dijit components I get the following error "lastChild is null or not an object".

I suggest the following patch to changeset 16227 :

<	for(var c; c = node.lastChild;){ // intentional assignment
<		d.destroy(c);
<	}
---
>	if (node) {
>		for(var c; c = node.lastChild;){ // intentional assignment
>			d.destroy(c);
>		}
>	}

comment:11 in reply to:  10 Changed 8 years ago by Eugene Lazutkin

Resolution: wontfix
Status: reopenedclosed

Replying to pavelpenchev:

Using IE7 and dojo 1.5.0 + some dijit components I get the following error "lastChild is null or not an object".

Dijit components should guard against this in their code --- we do not check against missing nodes intentionally. Closing this ticket for now. Please reopen with more information against affected components.

Note: See TracTickets for help on using tickets.