jQuery.buildFragment, ensure doc is a document. Fixes #8950 #347

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
5 participants
Member

rwaldron commented Apr 25, 2011

If someone could kindly review this.. thanks!

+ // Fixes #8950
+ if ( !doc.createDocumentFragment ) {
+ doc = document;
+ }
@timmywil

timmywil Apr 26, 2011

Owner

I wonder if we could combine this if with the doc definition above somehow. Maybe this is too fancy, but...

var node,
...
doc = ( nodes && (node = nodes[0])  && node.ownerDocument || node.createDocumentFragment && node ) || document;
Member

rwaldron commented Apr 26, 2011

@timmywil thanks for the review

rwaldron and others added some commits Apr 28, 2011

Merged pull request #361 from rwldrn/9008a.
Restored /g flag to rspaces; Adds unit tests; Supplements #9008
Fix #8985 use DocumentFragment get the default display value of an el…
…ement

1: Fix #8985 use DocumentFragment get the default display value of an element
src/manipulation.js
+ // ensure that the attr object doesnt incorrectly stand in as a document object
+ // Chrome and Firefox seem to allow this to occur and will throw exception. Fixes #8950
+ // For all other cases, declare and assign the proper document object
+ doc = ( nodes && (node = nodes[0]) && node.ownerDocument || node.createDocumentFragment && node ) || document;
@csnover

csnover Apr 30, 2011

Member

node.createDocumentFragment && node could easily be misinterpreted as a code bug (it took me several minutes to figure out that its intent is to ensure node is assigned and not simply that operands got inadvertently swapped); I’m strongly inclined to suggest that the added brevity is not worth the lack of clarity and that a more boring if/else structure should be used here. :)

@rwaldron

rwaldron Apr 30, 2011

Member

I agree - this execution is based on recommendations from another review. I'd also like to add comments to explain why this check happens (as you've noted). I should revert to my original patch and break the one liner out from there.

rwaldron added some commits Apr 30, 2011

Merge branch '8950' of github.com:rwldrn/jquery into 8950
* '8950' of github.com:rwldrn/jquery:
  Remove single space
  Fixes per review recommendations.
  jQuery.buildFragment, ensure doc is a document. Fixes #8950

@rwaldron rwaldron closed this Apr 30, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment