Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#9071 closed defect (fixed)

[regression] dojo.query matching too many elements

Reported by: dante Owned by: alex
Priority: high Milestone: 1.3.1
Component: Query Version: 1.3.0
Keywords: Cc:
Blocked By: Blocking:

Description

using a simple html set:

<div id="one">
    <p class="one"><a>one</a></p>
    <div id="two">
        <p class="two"><a>two</a></p>
    </div>
</div>

the query 'div p' returns 3 elements. also 'div p a'. there are clearly only two p and a elements in the page. the queryies 'p' and 'p a' both pass, as they don't contain an embedded match.

failing unit test attached.

Attachments (1)

query-quirk.html (830 bytes) - added by dante 11 years ago.
first two pass, second two fail

Download all attachments as: .zip

Change History (8)

Changed 11 years ago by dante

Attachment: query-quirk.html added

first two pass, second two fail

comment:1 Changed 11 years ago by bill

Milestone: tbd1.3.1
Summary: dojo.query matching too many elements[regression] dojo.query matching too many elements

This works in 1.2, fails in 1.3 (and trunk).

comment:2 Changed 10 years ago by nic

The test fails because there are two nested divs.
For example, dojo.query("div p"): the root.getElementsByTagName(query.getTag()) (query.js, line 1033) finds one node, p.two, when root is the inner div and two nodes, p.one and p.two again, when root is the outer div.
So the ret array contains three elements and the _zip function is never called (ret.nozip = true, line 1092).
Maybe the assumption root nodes > 0 => no dup.controls (line 1086) is correct only when there are no ancestor/descendant root nodes.

comment:3 Changed 10 years ago by James Burke

If ret.nozip is not needed on line 1092, then perhaps get rid of the if (x > 0) block also. From what I can tell, bag is never assigned otherwise in the filterDown function, and my cursory look at getElementsFunc and "bag" references seem to indicate bag is not modified underneath either. But it would be good to get confirmation from Alex on it.

comment:4 Changed 10 years ago by alex

Status: newassigned

comment:5 Changed 10 years ago by alex

(In [17460]) fix for query system matching too many elements (possibly not checking for uniqueness in a set). Refs #9071. Back-port to 1.3.x branch will fix. !strict

comment:6 Changed 10 years ago by alex

Resolution: fixed
Status: assignedclosed

(In [17461]) back-porting fix for query() DOM branch matching too many nodes in descendant selectors. Fixes #9071. !strict

comment:7 Changed 10 years ago by bill

Component: CoreQuery
Note: See TracTickets for help on using tickets.