Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#14803 closed defect (fixed)

[patch][cla] dojo.query() fails to match #c when there is no #b when given this query: "#a,#b,#c"

Reported by: Ray Waldin Owned by: Kris Zyp
Priority: high Milestone: 1.9
Component: Query Version: 1.7.2rc1
Keywords: Cc:
Blocked By: Blocking:

Description

A working example of this bug can be found here: http://jsfiddle.net/rwaldin/SEMHF/

dojo.query() will fail to match correctly when issued queries with multiple selector groups (i.e. "dojo.query('#a,#b,#c')") if there are no elements matching the second selector group (#b in this case), and the query is handled by acme and not document.querySelectorAll (e.g., in IE < 8, or in webkit/firefox with no spaces in the query and only ids).

This issue is a duplicate of #7125. That issue was incorrectly marked as fixed. The revision in which the supposed fix was committed (r16618) also changed the test case, circumventing the original problem.

The problem lies with the _zip() function, which tests the elements of its array parameter for truthiness. If any element beyond the first is falsy, this function short circuits and fails to process subsequent elements.

The attached patch, made against the 1.7.2rc1 tagged acme.js, fixes this problem. It also includes a test case that demonstrates the problem and the fix.

Attachments (2)

acmezipfix.patch (2.3 KB) - added by Ray Waldin 8 years ago.
acmezipfix-alternative.patch (1.6 KB) - added by Brandon Payton 7 years ago.
Alternative patch, under CLA and simpler than the original

Download all attachments as: .zip

Change History (12)

Changed 8 years ago by Ray Waldin

Attachment: acmezipfix.patch added

comment:1 Changed 8 years ago by bill

Owner: changed from Kris Zyp to Ray Waldin
Status: newpending
Summary: dojo.query() fails to match #c when there is no #b when given this query: "#a,#b,#c"[patch] dojo.query() fails to match #c when there is no #b when given this query: "#a,#b,#c"

Hi Ray. That's great that you have the patch, including a test case. We just need you to file a CLA so we can use it, or let us know if you are working for a company w/a CCLA (and which company it is), thanks!

comment:2 Changed 8 years ago by trac-o-bot

Resolution: invalid
Status: pendingclosed

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

comment:3 Changed 7 years ago by bill

Resolution: invalid
Status: closedreopened
Summary: [patch] dojo.query() fails to match #c when there is no #b when given this query: "#a,#b,#c"dojo.query() fails to match #c when there is no #b when given this query: "#a,#b,#c"

dojo/tests/query/query.html has doh.is(2, (query('#baz,#foo,#t')).length); which passes even on IE6. But changing it to query('#foo,#baz,#t') only returns only 1 element on IE8 and chrome.

So although we can't use Ray's patch, looks like there's a bug, so this ticket should be left open.

comment:4 Changed 7 years ago by bill

In [30687]:

test case for query(#a,#b,#c) when #b doesn't exist, commented out until bug fixed, refs #14803

Changed 7 years ago by Brandon Payton

Alternative patch, under CLA and simpler than the original

comment:5 Changed 7 years ago by Brandon Payton

Here is an alternative patch that fixes the issue and dispenses with pre-loop logic involving arr[0], using normal 0 .. length - 1 loops instead.

acme.js is slightly smaller after the patch.

comment:6 Changed 7 years ago by dylan

Milestone: tbd1.9
Summary: dojo.query() fails to match #c when there is no #b when given this query: "#a,#b,#c"[patch][cla] dojo.query() fails to match #c when there is no #b when given this query: "#a,#b,#c"

comment:7 Changed 7 years ago by dylan

Owner: changed from Ray Waldin to Kris Zyp
Priority: undecidedhigh
Status: reopenedassigned

comment:8 Changed 7 years ago by bill

Cool, that patch works for me (after uncommenting the test case I put into query.html).

comment:9 Changed 6 years ago by Kris Zyp

Resolution: fixed
Status: assignedclosed

In [31285]:

Fix unioned queries that contain no elements, fixes #14803 !strict

comment:10 Changed 6 years ago by Kris Zyp

In [31286]:

Uncomment test associated with refs #14803, !strict

Note: See TracTickets for help on using tickets.