Opened 13 years ago

Closed 12 years ago

Last modified 10 years ago

#2423 closed defect (fixed)

dojo.query picky vis a vis whitespace

Reported by: dojotoolkit@… Owned by: alex
Priority: high Milestone: 1.0
Component: Query Version: 0.9
Keywords: Cc: dojotoolkit@…, dylan
Blocked By: Blocking:

Description

Given:

<ul>
  <li class="myClass"><a href="#">Link</a></li>
</ul>

This works:

dojo.query('li.myClass a');

This fails (note the extra space):

dojo.query('li.myClass  a');

Change History (14)

comment:1 Changed 13 years ago by dylan

Milestone: 0.9
Owner: changed from anonymous to alex
Version: 0.9

to keep things lean and as fast as possible with this, I sort of think this is a good idea? Kicking to Alex for his thoughts on performance impact of normalizing whitespace on dojo.query queries.

comment:2 Changed 12 years ago by dylan

note that the following fails:

dojo.query('div,p');

whereas this works:

dojo.query('div, p');

People writing css are used to having their whitespace auto-normalized.

comment:3 Changed 12 years ago by dylan

(In [8724]) refs #2423, add test showing dojo.query whitespace pickiness

comment:4 Changed 12 years ago by dylan

Added a test.. here's the failure:

     _AssertFailure: http://dojo.localdomain/dojo/_base/_loader/bootstrap.js:281 doh._AssertFailure: assertEqual() failed: expected |2| but got |0|
 doh._AssertFailure
     ERROR IN:
  (function anonymous(t) {t.is(2, dojo.query(".foo,.bar").length);})

comment:5 Changed 12 years ago by dylan

more info in #3197

comment:6 Changed 12 years ago by bill

Component: DOMGeneral

comment:7 Changed 12 years ago by alex

(In [9059]) make query() more resiliant in whitespace parsing for compund queries. Refs #2423

comment:8 Changed 12 years ago by tk

Cc: dojotoolkit@… dylan added

Test appears to pass here: http://archive.dojotoolkit.org/nightly/dojotoolkit/dojo/tests/_base/query.html however, firebug isnt included in the page so not for sure how a failure would look...

Should this be closed now? ( I only have IE to work with here... so cant just check firebug in FF)

-Karl

comment:9 Changed 12 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

It seems this is fixed, from firebug I see:

PASSED test: t.is(2, (dojo.query('.foo, .bar')).length);

PASSED test: t.is(2, (dojo.query('.foo,.bar')).length);

comment:10 Changed 12 years ago by alex

Milestone: 0.91.0

there are still some nagging issues w/ whitespace in query() which aren't solved yet. I'm re-opening but targeting for 1.0.

comment:11 Changed 12 years ago by alex

Resolution: fixed
Status: closedreopened

comment:12 Changed 12 years ago by alex

Resolution: fixed
Status: reopenedclosed

(In [11174]) complete replacement for the tokenization strategy of the query engine. Moved from ad-hoc substring searching to an AST generation/consumption system. The query engine should be significantly more robust in the face of whitespace and funky formatting than before. Code size change should be minimal despite the large-scale change. Expanded tests and a first stab at inline docs for dojo.query. All unit tests pass on FF2, Safari3, and IE 6 and 7. Fixes #4365. Fixes #4640. Fixes #2423.

comment:13 Changed 12 years ago by alex

(In [11175]) re-enable query optimizer. Refs #2423

comment:14 Changed 10 years ago by bill

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