Opened 13 years ago
Closed 11 years ago
#6236 closed bug (wontfix)
jQuery incorreclty closes tags in the clean function when the last attribute value ends with a "/" character
Reported by: | rformato | Owned by: | |
---|---|---|---|
Priority: | low | Milestone: | 1.next |
Component: | manipulation | Version: | 1.6b1 |
Keywords: | ie6 ie7 ie8 | Cc: | |
Blocked by: | Blocking: |
Description
When adding HTML code retrieved with a call to html() with IE, because of the weirdness of IE parsed code, that is basically IE strip all the quotes around attribute values, the clean function may incorrectly close an element if the value of its last attribute ends with a "/" character.
test this in the IE8 console
$("<div>").html($("<div><form method=\"post\" action=\"./\"><div></div></form></div>").html()).html()
Attachments (1)
Change History (25)
Changed 13 years ago by
Attachment: | patch.diff added |
---|
comment:1 Changed 13 years ago by
Component: | unfiled → core |
---|
comment:2 Changed 12 years ago by
Milestone: | 1.4.3 |
---|---|
Priority: | → low |
Status: | new → open |
Summary: | jQuery incorreclty closes tags in the clean function with IE when the last attribute value ends with a "/" character → jQuery incorreclty closes tags in the clean function when the last attribute value ends with a "/" character |
Version: | 1.4.2 → 1.4.4rc |
So this is actually a bug in jQuery, it’s just exposed most easily by IE because it has a shitty HTML serializer.
HTML5 states that quotes are optional for strings that don’t contain any of "'!`=<> or spaces, but jQuery assumes that /> means someone passing in some XHTML with a self-closing tag and tries to “correct” it, which breaks.
comment:5 Changed 12 years ago by
FYI: in my case patch applied to 1.4.2 fixed the problem, but solution from 1.4.4 (line 4831) did not.
comment:7 Changed 12 years ago by
Here’s a simple test case that fails in all browsers: http://jsfiddle.net/mathias/9MgB4/ (from bug #8218)
comment:8 Changed 12 years ago by
The bug still exists in 1.4.4 . Any plans to get the patch into the mainstream ? This should be of higher priority.
comment:10 Changed 12 years ago by
Component: | core → manipulation |
---|---|
Version: | 1.4.4rc → 1.6b1 |
comment:12 Changed 12 years ago by
I'm unclear on what 1.next means. We are at 1.6.1 at the moment. Does this mean 1.7? 1.6.2?
comment:15 Changed 12 years ago by
For reference, start tags that end with "/>" are actually invalid, see: http://www.whatwg.org/specs/web-apps/current-work/#unquoted and http://www.whatwg.org/specs/web-apps/current-work/#syntax-start-tag
comment:17 Changed 12 years ago by
comment:19 Changed 12 years ago by
This bug still exists in 1.6.2 - is there any way it could get some love for the next release?
comment:21 Changed 11 years ago by
Just want to point out how evil this bug can be when using innerHTML:
http://jsfiddle.net/cmcnulty/Q2psh/
In that fiddle, I'm inserting a totally benign and compliant form tag, however, as someone else noted, IE, when using innerHTML *both* strips out the quotes *and* moves the action attribute to the last attr. This means that when you use innerHTML for any form tag where the action ends with a forward slash (pretty common), IE will helpfully insert an *invalid, non-compliant* string. If you then take the resulting innerHTML and do anything with it with jquery, you're going to have problems.
It's just a shame, because it should be a very rare problem, but IE's insanity makes it pretty common.
comment:22 Changed 11 years ago by
One more comment about this: we could fix the regex test easily for elements that have attributes by inserting [\s'"] into the rxhtmlTag test:
rxhtmlTag = /<(?!area|br|col|embed|hr|img|input|link|meta|param)(([\w:]+)[^>]*)[\s'"]\/>/ig,
and that will fix the <form action=google.com/> problem. However, it will break in the pretty common practice of inserting self-closing tags that shouldn't be, for instance:
$(foo).html('<p/>');
I'm not a regex pro though. It seems like it should be possible to come up with a test that will only match in cases where the closing "/>" is preceded with a singe/double quote or whitespace OR where the preceding characters are part of the opening tag.
Anyone want to take a shot at it?
comment:23 Changed 11 years ago by
I've thought about this one and vote against fixing it, for the following reasons:
$el.html( $other.html() )
is an odd approach... I'd bet that 95% of such uses should instead be$el.empty().append( $other.clone() )
- Fixing this would mean breaking unusual but technically valid (for jQuery) manual input like
$("<textarea name=description/>")
- The problem is not with our processing of purported HTML, it's IE's generation of invalid HTML from the DOM
- Legitimate use of the
.html( strInnerHTML )
pattern, if there is any, can insert preprocessing to ensure that all terminal attributes are quoted:.html( strInnerHTML.replace( // CDATA section; or // comment; or // element open tag (capturing assignment of an unquoted last attribute) /<!\[[^\0]*?\]\]>|<!-\-[^\0]*?-\->|(<[\w:](?:[^'">]*('|").*?\2)*[^>]*?)(=[^\s>]+)?>/g, // quote unquoted last attributes; return everything else unaltered function( tag, elementStart, _, unquoted ) { return unquoted ? elementStart + "='" + unquoted.substring(1) + "'>" : tag; } ) )
If anything, the above should be used to make .html()
always produce parseable output, but it's a substantial addition (regular expression plus function plus support test) with little benefit.
comment:24 Changed 11 years ago by
Keywords: | ie6 ie7 ie8 added |
---|---|
Resolution: | → wontfix |
Status: | open → closed |
I have to agree, this has stayed open so long because there is no reasonable fix.
proposed patch