Manipulation: Restrict the tbody search to child nodes #3463
Conversation
|
@gibson042, thanks for your PR! By analyzing the history of the files in this pull request, we identified @markelog, @timmywil and @rwaldron to be potential reviewers. |
src/manipulation.js
Outdated
| jQuery.nodeName( content.nodeType !== 11 ? content : content.firstChild, "tr" ) ) { | ||
|
|
||
| return elem.getElementsByTagName( "tbody" )[ 0 ] || elem; | ||
| return jQuery.filter( "tbody", elem.childNodes )[ 0 ] || elem; |
There was a problem hiding this comment.
Clever, maybe a comment though? You know I like comments :)
There was a problem hiding this comment.
Hmm, what are you looking for that's not covered by the new function comment? Should I update it to something like "New rows for a table belong in its child tbody when one is present", or move it above this line, or something else?
There was a problem hiding this comment.
Actually, the code itself looks pretty clear and definitely has the benefit of being more correct. Funny that nobody thought about nested tables for this case.
There was a problem hiding this comment.
It only comes up when the target doesn't have a tbody but a descendant does, or when such a descendant is in the thead, both of which I imagine are rare. And the logic itself goes waaaaaay back.
|
@timmywil just FYI (in case you didn't know that): you can now approve pull requests, it seems a bit better then "reaction" :) |
Exercise a code path that uses querySelectorAll instead of Javascript iteration. http://codepen.io/anon/pen/vywJjx?editors=1010
5 participants
Fixes gh-3439