.find breaks with documentfragments

Reported by:	sahbeewah@gmail.com	Owned by:
Priority:	low	Milestone:	None
Component:	selector	Version:	2.0b2
Keywords:		Cc:	dmethvin, timmywil
Blocked by:		Blocking:

Description

Unexpected behaviour when dealing with DocumentFragments...

var $d = $(document.createDocumentFragment());
$d.append($('<div><a>wee</a></div>')).append($('<p>my paragraph</p>'));

var response;
response = 'I found ' + $d.children().length + ' children.<br>';
response += 'I found ' + $d.children('div').length + ' div children.<br>';
$d.children().each(function() { 
    var escapedChildren = this.outerHTML.replace(/</g, '<').replace(/>/g, '>');
    response += 'Child: ' + escapedChildren + '<br>';
});
response += 'But ' + $d.find('a').length + ' As using find...<br>';
response += 'But ' + $d.find('div').length + ' DIVs using find...<br>';
response += 'Wat?';
$('#report').html(response);

returns:

I found 2 children.
I found 1 div children.
Child: <div><a>wee</a></div>
Child: <p>my paragraph</p>
But 0 As using find...
But 0 DIVs using find...
Wat?

Tested on Chrome 24.0.1312.70 on Debian

Attachments (0)

Change History (9)

Changed March 01, 2013 01:48PM UTC by sahbeewah@gmail.com comment:1

Please follow the bug reporting guidlines and use jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

My Bad.

http://jsfiddle.net/w7KFN/

Changed March 05, 2013 05:13AM UTC by bjohn465 comment:2

.find() uses Sizzle, which appears to always return an empty array when a document fragment is used:

https://github.com/jquery/sizzle/blob/4bd24fccc7c00191d57c09f111b074f516203369/sizzle.js#L239-L241

In the test case, when this bit of Sizzle code is executed, context is the document fragment, which has a nodeType of 11, therefore zero results are returned.

Looking further down in sizzle.js, it appears document fragments are excluded because they don't have some methods used by the shortcuts, such as getElementById (line 250), getElementsByClassName (line 278), and, what would probably be used in the given test case if this were not a document fragment, getElementsByTagName (line 274). There could be other methods Sizzle expects from context that document fragments do not include.

Changed March 05, 2013 01:57PM UTC by gibson042 comment:3

cc:	→ dmethvin, timmywil
component:	unfiled → selector

Absolutely correct. And looking at that block of code with fresh eyes, I think it's due for some tweaking... instead of returning empty for an invalid context, we ought to throw an exception.

Changed March 05, 2013 03:24PM UTC by dmethvin comment:4

I'm not a big fan of throwing exceptions but it seems appropriate for this case since it emphasizes that document fragments are not supported.

Changed March 13, 2013 01:09AM UTC by timmywil comment:5

priority:	undecided → low
status:	new → open
version:	git → 2.0b2

Let's not throw an exception. Consistency is the issue. I'm not sure which way to lean for consistency though.

Changed April 29, 2013 03:51PM UTC by dmethvin comment:6

I should also point out here that document fragments are not considered to be valid input to any of the jQuery APIs and we don't have test cases for them. So "notabug" could also be the correct answer, and if we're going to fix this we'll need a lot more test cases.

Changed May 02, 2013 09:10PM UTC by timmywil comment:7

resolution:	→ wontfix
status:	open → closed

I don't think we should make a change here. Sometimes document fragments need to work (because we created them) and sometimes they don't (because we didn't). ;) Besides, document fragments aren't supported. I don't think we need to _ensure_ no support for them.

Changed May 07, 2013 04:09PM UTC by gibson042 comment:8

Moved to Sizzle: https://github.com/jquery/sizzle/issues/210

Changed March 06, 2014 10:56PM UTC by tenbits comment:9

Replying to [comment:7 timmywil]: