Opened 10 years ago
Closed 10 years ago
#13077 closed bug (wontfix)
IE6-8 can't insert html comment with .html()
Reported by: | cmcnulty | Owned by: | |
---|---|---|---|
Priority: | low | Milestone: | 1.9 |
Component: | manipulation | Version: | git |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
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.
Change History (8)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
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.
comment:3 Changed 10 years ago by
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.
comment:4 Changed 10 years ago by
@cmcnulty do you want to submit a patch for this? We'll need a unit test which will go into 2.0 as well.
comment:5 Changed 10 years ago by
Component: | unfiled → manipulation |
---|---|
Milestone: | None → 1.9 |
Priority: | undecided → low |
comment:6 Changed 10 years ago by
Status: | new → open |
---|
comment:7 Changed 10 years ago by
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.
comment:8 Changed 10 years ago by
Resolution: | → wontfix |
---|---|
Status: | open → closed |
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.
Is there a reason to let people do this? We have gone 6 years without it and nobody requested it.