Bug Tracker

Opened 10 years ago

Closed 7 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)

patch.diff (874 bytes) - added by rformato 10 years ago.
proposed patch

Download all attachments as: .zip

Change History (25)

Changed 10 years ago by rformato

Attachment: patch.diff added

proposed patch

comment:1 Changed 10 years ago by dmethvin

Component: unfiledcore

comment:2 Changed 9 years ago by snover

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

comment:3 Changed 9 years ago by snover

#5998 is a duplicate of this ticket.

comment:4 Changed 9 years ago by snover

#4417 is a duplicate of this ticket.

comment:5 Changed 9 years ago by shcodenick

FYI: in my case patch applied to 1.4.2 fixed the problem, but solution from 1.4.4 (line 4831) did not.

comment:6 Changed 9 years ago by jitter

#8218 is a duplicate of this ticket.

comment:7 Changed 9 years ago by mathias

Here’s a simple test case that fails in all browsers: http://jsfiddle.net/mathias/9MgB4/ (from bug #8218)

comment:8 Changed 9 years ago by shreekantbohra@…

The bug still exists in 1.4.4 . Any plans to get the patch into the mainstream ? This should be of higher priority.

comment:9 Changed 9 years ago by Shree Kant <shreekantbohra@…>

and in 1.5.2 as well.

comment:10 Changed 9 years ago by timmywil

Component: coremanipulation
Version: 1.4.4rc1.6b1

comment:11 Changed 9 years ago by timmywil

Milestone: 1.next

Confirmed

comment:12 Changed 9 years ago by alt255@…

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:13 Changed 9 years ago by Rick Waldron

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

comment:14 Changed 8 years ago by john

Confirmed during bug triage.

comment:15 Changed 8 years ago by Rick Waldron

comment:16 Changed 8 years ago by Rick Waldron

#9530 is a duplicate of this ticket.

comment:18 Changed 8 years ago by john

#8939 is a duplicate of this ticket.

comment:19 Changed 8 years ago by leoshklo

This bug still exists in 1.6.2 - is there any way it could get some love for the next release?

comment:20 Changed 8 years ago by dmethvin

#11576 is a duplicate of this ticket.

comment:21 Changed 8 years ago by cmcnulty

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 8 years ago by cmcnulty

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 8 years ago by gibson042

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.

Last edited 8 years ago by gibson042 (previous) (diff)

comment:24 Changed 7 years ago by dmethvin

Keywords: ie6 ie7 ie8 added
Resolution: wontfix
Status: openclosed

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

Note: See TracTickets for help on using tickets.