Skip to main content

Bug Tracker

Side navigation

#14290 closed bug (fixed)

Opened August 23, 2013 11:14PM UTC

Closed September 19, 2013 02:27PM UTC

regression $('<table></table>').append("text content") fails in 1.10

Reported by: karger@mit.edu Owned by: karger@mit.edu
Priority: undecided Milestone: None
Component: unfiled Version: 1.10.2
Keywords: Cc:
Blocked by: Blocking:
Description

$('<table></table>').append("text content") generates an exception in jquery 1.10. It works fine in jquery 1.9 . The resulting html is invalid, but it still seems wrong for jquery to throw an exception.

jsfiddle: http://jsfiddle.net/EPJS6/

The culprit is line 6300 of jquery-1.10.2.js ,

jQuery.nodeName( content.nodeType === 1 ? content : content.firstChild, "tr" ) 

which for some reason takes the firstChild of any content that is not an elementnode, so in particular when content is a textnode it takes the textNode's firstChild which is null and passes it as an argument to nodeName. Are the two cases of the conditional swapped from what they should be?

Attachments (0)
Change History (7)

Changed August 24, 2013 01:00AM UTC by dmethvin comment:1

I'm okay with it throwing an error for this case. Is there some benefit to developers to letting it quietly create invalid markup?

Changed August 24, 2013 03:46AM UTC by anonymous comment:2

Well, to be clearer, while it always crashes it only *sometimes* creates invalid markup. I ran into the bug because I am using a templating library that recursively copies child elements into (a copy of) the parent element. The bug was triggered when the templating engine was copying a template containing

<table>     <tbody><tr>stuff...</table>
. The first child the templating engine tried to copy into a new table node was the textnode consisting of just whitespace before the tbody. Creating (temporarily) <table>..whitespace..</table> is, I think, a plausible step in building a table. If jq continues to throw this error, then it becomes the responsibility of the template engine to check for and skip inserting whitespace, which just doesn't seem like a job it should have to do given that it is building valid html.

Changed August 30, 2013 11:57AM UTC by jordifebrer comment:3

_comment0: Ideally do not throw an exception and add the text content like jQuery 1.9 and older versions. \ \ Possible fix is: \ \ function manipulationTarget( elem, content ) { \ return jQuery.nodeName( elem, "table" ) && \ - jQuery.nodeName( content.nodeType === 1 ? content : content.firstChild, "tr" ) ? \ + jQuery.nodeName( ( content.nodeType === 1 || content.firstChild === null ) ? content : content.firstChild, "tr" ) 1377863960254700

Ideally do not throw an exception and add the text content like jQuery 1.9 and older versions.

Possible fix is:

function manipulationTarget( elem, content ) {
   return jQuery.nodeName( elem, "table" ) &&
-    jQuery.nodeName( content.nodeType === 1 ? content : content.firstChild, "tr" ) ?
+    jQuery.nodeName( ( content.nodeType === 1 || content.firstChild === null ) ? content : content.firstChild, "tr" )

Changed September 10, 2013 07:18PM UTC by dmethvin comment:4

owner: → karger@mit.edu
status: newpending

Sorry, what I was asking is, other than not throwing an error, what should the outcome be? You're defining a behavior with your suggested patch but is that really the desired behavior?

Changed September 12, 2013 06:13PM UTC by karger comment:5

Replying to [comment:4 dmethvin]:

Sorry, what I was asking is, other than not throwing an error, what should the outcome be? You're defining a behavior with your suggested patch but is that really the desired behavior?

So at the risk of overgeneralizing, I think we should recognize that it is possible (and natural) for invalid html to "grow" to become valid. This implies that jquery should be robust in handling (temporarily) invalid html, in order to protect the user from having to maintain validity at every single step.

Another, related example of this would be manipulating a table without a tbody. jquery appears to helpfully create a tbody in this case. But what happens if the table is only temporarily without a tbody, and the user adds one later? Suddenly we have two tbodys, which is a problem.

Changed September 13, 2013 05:31PM UTC by dmethvin comment:6

So at the risk of overgeneralizing, I think we should recognize that it is possible (and natural) for invalid html to "grow" to become valid. This implies that jquery should be robust in handling (temporarily) invalid html, in order to protect the user from having to maintain validity at every single step.

It can be risky to define our behavior as a superset of the standard. If the standard changes against jQuery's behavior, we're forced to choose between standards and backwards compatibility.

The pull request, only ensures that there is one child element in the table and (implicitly) that no exception was thrown. I think that is good, actually, because it doesn't put too many constraints on how a browser could handle invalid markup. But what if the browser discarded the text? I haven't looked at the requirements for how HTML5 would parse this but I wouldn't want to go beyond (and certainly not against) that.

Another, related example of this would be manipulating a table without a tbody. jquery appears to helpfully create a tbody in this case.

That is required because IE auto-inserts a tbody if we don't. It is easier to make all browsers behave consistently that way. I don't think this case is analogous.

But what happens if the table is only temporarily without a tbody, and the user adds one later? Suddenly we have two tbodys, which is a problem.

A table can have multiple tbody elements. http://dev.w3.org/html5/html-author/#the-table-element

Changed September 19, 2013 02:27PM UTC by Dave Methvin comment:7

resolution: → fixed
status: pendingclosed

Fix #14290. Don't throw if text node is appended to table. Close gh-1371.

Changeset: ec3ac9a247ec3132dea3c30ca36cefc217e25441