Opened 12 years ago

Closed 11 years ago

#4425 closed defect (fixed)

[patch][ccla] Query doesn't handle queries by ID relative to a root node properly

Reported by: guest Owned by: alex
Priority: high Milestone: 1.3
Component: Core Version: 0.9
Keywords: Cc: ben.schell@…
Blocked By: Blocking:

Description (last modified by Dustin Machi)

Lets say I have an HTML DOM element that does not belong to the current rendered page (maybe it was fetched from a remote page, or created programmatically). I then want to do some querying/parsing over this node and its children. Most operations of dojo.query (querying based on class, etc.) work properly. However, attempting to query based on id fails, as dojo.query uses dojo.byId (and therefore document.getElementById) to retrieve those elements.

If the purpose of dojo.query is to always fetch items relative to the provided root element, this is not the desired behavior. I've attached a sample test case, as well as a patch to dojo/_base/query.js which does find the appropriate element.

The source attached falls under the same CLA as Jared Jurkiewicz and Bill Keese.

Attachments (4)

queryTest.html (680 bytes) - added by guest 12 years ago.
test.html (71 bytes) - added by guest 12 years ago.
query.js.diff (1.7 KB) - added by guest 12 years ago.
query.js.2.diff (1.8 KB) - added by guest 12 years ago.

Download all attachments as: .zip

Change History (19)

Changed 12 years ago by guest

Attachment: queryTest.html added

Changed 12 years ago by guest

Attachment: test.html added

Changed 12 years ago by guest

Attachment: query.js.diff added

Changed 12 years ago by guest

Attachment: query.js.2.diff added

comment:1 Changed 12 years ago by guest

Another possible solution to this bug is allowing the caller to provide a function for finding elements based on ID, passed as an optional argument to dojo.query, like so:

function myById(id, root){
    //...Do Stuff...
    return node;
}
dojo.query("testId", div, myById);

This would obviously be a small API change by adding in an optional argument to dojo.query, but it would take the extra code (size and performance) out of core and require the developer to account for this (not-so-)special case of addressing DOM elements that aren't in the current document.

comment:2 Changed 12 years ago by Adam Peller

Owner: changed from anonymous to alex

comment:3 Changed 12 years ago by dylan

Milestone: 1.1
Summary: Query doesn't handle queries by ID relative to a root node properly[patch][ccla] Query doesn't handle queries by ID relative to a root node properly

comment:4 Changed 12 years ago by bill

Milestone: 1.11.2

Move all milestone 1.1 tickets to 1.2, except for reopened tickets and tickets opened after 1.1RC1 was released.

comment:5 Changed 11 years ago by Dustin Machi

Description: modified (diff)
Type: defectenhancement

comment:6 Changed 11 years ago by alex

Type: enhancementdefect

hrm, I think this really is a defect. It's a spec violation.

comment:7 Changed 11 years ago by alex

Status: newassigned

comment:8 Changed 11 years ago by liucougar

btw, this works as expected:

dojo.query('*[id="search_id"]',your_root_node)

comment:9 Changed 11 years ago by bill

Milestone: 1.21.3

as per today's meeting, punting these core bugs

comment:10 Changed 11 years ago by bill

Milestone: 1.3future

comment:11 Changed 11 years ago by alex

(In [16218]) some require() and NodeList? doc changes that happened while I was hacking on steak. Refs #7072, #4425

comment:12 Changed 11 years ago by alex

(In [16226]) steak -> acme. Need a better name. Refs #7072. Refs #4425. !strict

comment:13 Changed 11 years ago by alex

(In [16228]) updating for the acme engine. Refs #7072. Refs #4425

comment:14 Changed 11 years ago by alex

(In [16232]) some size reductions. Also (finally) enables the QSA branch. Not sure how I missed that.

Refs #7072. Refs #4425. !strict

comment:15 Changed 11 years ago by alex

Milestone: future1.3
Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.