IE6-8 can't insert html comment with .html()

[cmcnulty]

In IE6-8, see: ​http://jsfiddle.net/B7zQb/

This is the same oldIE noScope "feature" that affects the other elements in the rnoInnerhtml regex. Solution should be to simply to add !-- to the regex. Pull request coming soon.

[dmethvin]

Is there a reason to let people do this? We have gone 6 years without it and nobody requested it.

[cmcnulty]

Only academic reasons I suppose. But then the fix should only add 4 characters, and makes it works consistently across every other browser, and it's a result of a well-understood issue ("noScope" elements) for which we have a fix that works for the other elements that the issue affects (link, style and script).

To be honest though, when I have time, I'd like to take a little more time to take a crack at one last optimization of the way we handle the "innerHTML" shortcut in .html(), which is what this is all about, really.

On a related note, again, when I have time, I'd like to see if I can't prove that the entire innerHTML shortcut, and all of the work-arounds that are required to support it, are still worth it in jQ2.0. In other words, what's the performance ramifications of never doing an innerHTML for IE9.

[dmethvin]

Haha, cmcnulty, you got a fevah, and the only ​prescription is more 2.0!

rwaldron and gibson042 have manipulation.js in their sights, at least for the first pass, but I bet there's still room.

There's also the question of whether 2.0 can use createContextualFragment and/or insertAdjacentHTML for some of these cases, see #10426.

[dmethvin]

@cmcnulty do you want to submit a patch for this? We'll need a unit test which will go into 2.0 as well.

[cmcnulty]

There are two fixes, the first of which I actually hate, upon further reflection. It is to add "!--" to the rnoInnerhtml regex. I don't like this because it will force *any* html fragment that has a comment *anywhere* in it to use the much slower (in old ie) append method, instead of innerhtml. So in practice I believe there could be a perf degradation for many implementations with that fix.

The second which I kinda like, but which probably isn't wise, is to just go ahead and "fix" the problem in all cases by, basically, applying a mini-wrapMap fix while in the innerHTML code path. The problem with that is that unlike in the clean function where the wrapMap fix is applied to a non-attached node, in the innerHTML path, the fix would be applied to a (potentially) attached node.

The code is basically to prepend the innerHTML value with the shim that fixes the issue: "<div>x</div>" and then immediately remove the firstchild, which should clean up the fix. It actually passes all unit tests. But I don't think it's an acceptable fix because of the potential for redrawing the screen an extra time as it manipulates an attached element.

So, in short, I guess I recommend this as wontfix.

[dmethvin]

I have no problem closing this without a fix, it seems like a pretty rare case to me anyway and I don't think there is a practical impact on real code. Thanks for thinking it through, at least we have this ticket now as a record.