Bug Tracker

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#11473 closed bug (wontfix)

$.fn.html clears children in IE

Reported by: braddunbar Owned by:
Priority: undecided Milestone: None
Component: unfiled Version: git
Keywords: Cc: dmethvin
Blocked by: Blocking:

Description

When using innerHTML to set the contents of an element, descendant elements are also altered.

Observed using IE9 on Windows 7.

http://jsfiddle.net/Hej6h/6/

This is not a jQuery bug per se, but something that I think jQuery should handle if possible.

Change History (20)

comment:1 Changed 5 years ago by rwaldron

  • Cc dmethvin added

My initial thoughts is that this isn't a bug. If you empty the contents of a node, there is nothing for an existing reference to point to.

comment:2 Changed 5 years ago by scottgonzalez

I wouldn't expect any specific behavior of the child elements other than cleaning up any data stored by jQuery to avoid leaks. What specifically is this affecting that you think needs to be addressed?

Last edited 5 years ago by scottgonzalez (previous) (diff)

comment:3 Changed 5 years ago by rwaldron

  • Resolution set to invalid
  • Status changed from new to closed

The correct way to preserve anything you want to reuse is to store the actual innerHTML value in a variable - otherwise all nodes and their references will be cleared.

http://jsfiddle.net/rwaldron/DwusY/

comment:4 Changed 5 years ago by braddunbar

I agree, all nodes and their references will be cleared. However, their innerHTML should not be altered. For instance, if element c is a child of element p, calling p.html('') should leave the innerHTML of c unaltered. Is this not correct?

var p = $('<p><a>x</a></p>');
var c = p.find('a');
p.html('');
console.log(c.html()); //should produce 'x'
Last edited 5 years ago by braddunbar (previous) (diff)

comment:5 Changed 5 years ago by braddunbar

For what it's worth, the above is not the current behavior in IE (http://jsfiddle.net/BG45f/). In any case, jQuery should behave consistently.

comment:6 Changed 5 years ago by scottgonzalez

Brad makes a good point. The behavior of using innerHTML in IE seems incorrect.

comment:7 Changed 5 years ago by braddunbar

Also, a proposed solution is here: https://github.com/jquery/jquery/pull/708

comment:8 Changed 5 years ago by scottgonzalez

I just discussed this with Hixie and he agrees that this is a bug in IE. I'll report it to Microsoft and see what they say about it.

comment:9 Changed 5 years ago by dmethvin

  • Resolution invalid deleted
  • Status changed from closed to reopened

comment:10 Changed 5 years ago by dmethvin

I saw the pull request and agree the behavior described by @braddunbar above should be what IE does as well. We can make it work that way, but it seems like the performance penalty will be pretty high to do it. That concerns me. Given how this hasn't been reported before, I have to think it's not a common use case.

comment:11 Changed 5 years ago by rwaldron

Existing non-jQuery discussion, re: lost child nodes as a result of innerHTML being set: http://stackoverflow.com/questions/595808/is-it-possible-to-append-to-innerhtml-without-destroying-descendants-onclick-fu

Latest innerHTML spec: (attention to Step #3) http://www.w3.org/TR/html5/apis-in-html-documents.html#innerhtml

Previous innerHTML spec: (attention to Step #2) http://www.w3.org/TR/2008/WD-html5-20080610/dom.html#innerhtml0

comment:12 Changed 5 years ago by dmethvin

@rwaldron is cheating by quoting the specs that could be interpreted to allow the IE behavior. :)

I'll leave this open for further discussion, but am still leaning towards not trying to fix it. Someone who wants to depend on the behavior can use .empty().append(html) explicitly and get consistent cross-browser behavior. We could lobby for Microsoft to make the behavior consistent with the other browsers in IE10 though.

comment:13 Changed 5 years ago by braddunbar

I understand that the performance penalty is steep and perhaps makes fixing this issue a non-starter. Given the obscurity of the problem it seems like a valid trade-off. If this is the case, I think the side effects of .html() should certainly be documented so that new users can make an informed decision about .empty().append(...) vs .html(...). Also, how can I help lobby to Microsoft for a consistent behavior in IE10?

comment:14 follow-up: Changed 5 years ago by rkatic

the performance penalty is steep

Not sure about that. Am I missing something?

comment:15 in reply to: ↑ 14 Changed 5 years ago by rwaldron

Replying to rkatic:

the performance penalty is steep

Not sure about that. Am I missing something?

The penatly of looping through and element's child nodes and removing them one at a time? There has to be a reflow in there...

comment:16 Changed 5 years ago by braddunbar

Yes, essentially it changes the time complexity of that portion of the function from O(1) to O(# of child nodes). In some casual testing I did I saw a performance hit of 60% or more.

comment:17 Changed 5 years ago by dmethvin

  • Resolution set to wontfix
  • Status changed from reopened to closed

And it makes the performance worse for the browsers that have the worst performance already. IMO it would be better to wave people away from expecting the "correct" behavior in IE if they try to use .html(). And again it's easy to fix using .empty().append(...) so it's not as if the workaround is horrendous. I've updated the docs to note this issue.

We've contacted Microsoft about this issue and will try to get them to deal with it. Since it's a functional change it isn't likely to happen until IE10 at the earliest, unless you can prove a security problem is exposed by it. :)

comment:18 Changed 5 years ago by braddunbar

Sounds good. Thanks @dmethvin!

comment:19 Changed 5 years ago by rkatic

I would prefer correctness/consistency over performances, even in this case. Performance penalty is not that high, IMO. However, I can understand that fixing this could rise some revolt...

comment:20 Changed 5 years ago by scottgonzalez

Microsoft's response is that they want to fix this, but it's a very large change to the underlying code and therefore won't make it in to IE 10. Removing the children, as in the PR, is their recommended workaround. However, they have the same opinion as Dave: this should not be addressed by jQuery because of the performance hit.

Note: See TracTickets for help on using tickets.