Side navigation
#6236 closed bug (wontfix)
Opened March 06, 2010 01:51PM UTC
Closed June 26, 2012 02:24AM UTC
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 (24)
Changed June 12, 2010 03:25AM UTC by comment:1
component: | unfiled → core |
---|
Changed November 04, 2010 01:36AM UTC by comment:2
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.
Changed January 09, 2011 11:28PM UTC by comment:5
FYI: in my case patch applied to 1.4.2 fixed the problem, but solution from 1.4.4 (line 4831) did not.
Changed February 09, 2011 12:48PM UTC by comment:7
Here’s a simple test case that fails in all browsers: http://jsfiddle.net/mathias/9MgB4/ (from bug #8218)
Changed April 10, 2011 05:16AM UTC by comment:8
The bug still exists in 1.4.4 . Any plans to get the patch into the mainstream ? This should be of higher priority.
Changed April 10, 2011 05:24AM UTC by comment:9
and in 1.5.2 as well.
Changed April 17, 2011 05:05PM UTC by comment:10
component: | core → manipulation |
---|---|
version: | 1.4.4rc → 1.6b1 |
Changed April 17, 2011 05:43PM UTC by comment:11
milestone: | → 1.next |
---|
Confirmed
Changed June 15, 2011 06:14PM UTC by comment:12
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?
Changed June 15, 2011 06:57PM UTC by comment:13
It means 1.next-release-with-no-specific-deadline
Changed July 12, 2011 03:41PM UTC by comment:14
Confirmed during bug triage.
Changed July 12, 2011 03:57PM UTC by comment:15
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
Changed July 12, 2011 05:42PM UTC by comment:16
#9530 is a duplicate of this ticket.
Changed July 12, 2011 05:57PM UTC by comment:17
Changed July 12, 2011 07:55PM UTC by comment:18
#8939 is a duplicate of this ticket.
Changed August 08, 2011 08:44PM UTC by comment:19
This bug still exists in 1.6.2 - is there any way it could get some love for the next release?
Changed April 10, 2012 01:29PM UTC by comment:20
#11576 is a duplicate of this ticket.
Changed April 13, 2012 06:13PM UTC by comment:21
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.
Changed April 13, 2012 06:51PM UTC by comment:22
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?
Changed April 15, 2012 01:14PM UTC by comment:23
_comment0: | 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: \ {{{#!js \ .html( strInnerHTML.replace( \ // CDATA section; or \ // comment; or \ // element open tag (capturing assignment of an unquoted last attribute) \ /<!\\[[^\\0]*?\\]\\]>|<!-\\-[^\\0]*?-\\->|(<[^!](?:[^'">]*('|").*?\\2)*[^>]*?)(=[^\\s>]+)?>/g, \ 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. → 1334495970310593 |
---|---|
_comment1: | 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: \ {{{#!js \ .html( strInnerHTML.replace( \ // CDATA section; or \ // comment; or \ // element open tag (capturing assignment of an unquoted last attribute) \ /<!\\[[^\\0]*?\\]\\]>|<!-\\-[^\\0]*?-\\->|(<[^!\\/](?:[^'">]*('|").*?\\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. → 1334496028142482 |
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.
Changed June 26, 2012 02:24AM UTC by comment:24
keywords: | → ie6 ie7 ie8 |
---|---|
resolution: | → wontfix |
status: | open → closed |
I have to agree, this has stayed open so long because there is no reasonable fix.