Skip to main content

Bug Tracker

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)
  • patch.diff (0.9 KB) - added by rformato March 08, 2010 09:43AM UTC.

    proposed patch

Change History (24)

Changed June 12, 2010 03:25AM UTC by dmethvin comment:1

component: unfiledcore

Changed November 04, 2010 01:36AM UTC by snover comment:2

milestone: 1.4.3
priority: → low
status: newopen
summary: jQuery incorreclty closes tags in the clean function with IE when the last attribute value ends with a "/" characterjQuery incorreclty closes tags in the clean function when the last attribute value ends with a "/" character
version: 1.4.21.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.

improved test case

Changed November 22, 2010 05:16AM UTC by snover comment:3

#5998 is a duplicate of this ticket.

Changed November 22, 2010 05:19AM UTC by snover comment:4

#4417 is a duplicate of this ticket.

Changed January 09, 2011 11:28PM UTC by shcodenick 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:21PM UTC by jitter comment:6

#8218 is a duplicate of this ticket.

Changed February 09, 2011 12:48PM UTC by mathias 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 shreekantbohra@gmail.com 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 Shree Kant <shreekantbohra@gmail.com> comment:9

and in 1.5.2 as well.

Changed April 17, 2011 05:05PM UTC by timmywil comment:10

component: coremanipulation
version: 1.4.4rc1.6b1

Changed April 17, 2011 05:43PM UTC by timmywil comment:11

milestone: → 1.next

Confirmed

Changed June 15, 2011 06:14PM UTC by alt255@gmail.com 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 rwaldron comment:13

It means 1.next-release-with-no-specific-deadline

Changed July 12, 2011 03:41PM UTC by john comment:14

Confirmed during bug triage.

Changed July 12, 2011 03:57PM UTC by rwaldron comment:15

Changed July 12, 2011 05:42PM UTC by rwaldron comment:16

#9530 is a duplicate of this ticket.

Changed July 12, 2011 07:55PM UTC by john comment:18

#8939 is a duplicate of this ticket.

Changed August 08, 2011 08:44PM UTC by leoshklo 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 dmethvin comment:20

#11576 is a duplicate of this ticket.

Changed April 13, 2012 06:13PM UTC by cmcnulty 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 cmcnulty 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 gibson042 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 dmethvin comment:24

keywords: → ie6 ie7 ie8
resolution: → wontfix
status: openclosed

I have to agree, this has stayed open so long because there is no reasonable fix.