Opened 11 years ago

Closed 10 years ago

Last modified 7 years ago

#8775 closed defect (fixed)

REGRESSION: Camel Cased classname queries fail on Safari 3.2.2, Chrome, FF 3.5

Reported by: Jared Jurkiewicz Owned by: alex
Priority: high Milestone: 1.3.2
Component: Core Version: 1.3.0b2
Keywords: Cc: Nathan Toone
Blocked By: Blocking:

Description (last modified by Jared Jurkiewicz)

Camel Cased classname queries fail on Safari 3.2.2 and Chrome

I was updating one of my applications to dojo 1.3 to test it out and hit this issue. I had a place where I was querying:

dojo.query(".resortList);

Which looked for that class on a series of tags. That is the exact classname I used, complete with casing.

On FireFox? 2, IE, etc, I would get back a nodelist of 3 entries. On Safari 3.2.2 and Chrome, I get back 0

Now, If I query it as:

dojo.query(".resortlist);

It returned 3 on Safari 3.2.2 and Chrome.

CSS Spec denotes class selectors are supposed to be case insensitive ... so the camel case one should work .. but for those browsers it does not.

In checking the query UT, I noticed that the query UT doesn't do any camel-cased selector querying, which would have caught this error. I'll be providing an updated test set with one to show the failure shortly.

Given this used to work (in dojo 1.2.3 it does on the same browser), This is a regression on 1.3, and pretty major (IMHO), with regard to Safari/Webkit? support.

Attachments (3)

query.html (10.7 KB) - added by Jared Jurkiewicz 11 years ago.
Updated Query UT file with a Camelcase check. Passes on FF, Opera, fails on WebKit? (Safari 3.2.2, Chrome)
8775.patch (1.3 KB) - added by Douglas Hays 11 years ago.
possible fix that calls toLowerCase on class names if webkit/QSA
camelquery.php (1.2 KB) - added by dante 10 years ago.
small test to toggle quirksmode and show some numbers.

Download all attachments as: .zip

Change History (37)

Changed 11 years ago by Jared Jurkiewicz

Attachment: query.html added

Updated Query UT file with a Camelcase check. Passes on FF, Opera, fails on WebKit? (Safari 3.2.2, Chrome)

comment:1 Changed 11 years ago by Jared Jurkiewicz

Summary: Camel Cased classname queries fail on Safary 3.2.2 and ChromeREGRESSION: Camel Cased classname queries fail on Safary 3.2.2 and Chrome

I ran the exact same test on Dojo 1.2.3 on the same browser (Safari 3.2.2), and it passes. Definitely a regression.

comment:2 Changed 11 years ago by alex

Status: newassigned

this is very odd, mostly because what you've said implies that it's a querySelectorAll() bug. It *could* happen, but it would be strange to see something this bad actually ship. Which version of Chrome did you try this on? And what happens under a webkit nightly?

comment:3 Changed 11 years ago by Jared Jurkiewicz

Safari: 3.2.2 (525.28.1)

Chrome: 1.0.154.48

Haven't tried the latest WebKit? nightlies.

comment:4 Changed 11 years ago by Jared Jurkiewicz

This problem doesn't occur on Safari 4 beta (Build 528.16)

comment:5 Changed 11 years ago by Jared Jurkiewicz

So ... it probably is a querySelectorAll WebKit? bug in the versions of WebKit? Safari 3.2.2 and Chrome include. Ack.

comment:6 Changed 11 years ago by Jared Jurkiewicz

Description: modified (diff)

comment:7 Changed 11 years ago by bill

Milestone: tbd1.3

I think everyone will agree this needs to be fixed for 1.3, so marking as such.

Changed 11 years ago by Douglas Hays

Attachment: 8775.patch added

possible fix that calls toLowerCase on class names if webkit/QSA

comment:8 Changed 11 years ago by alex

(In [16920]) test case. Refs #8775

comment:9 Changed 11 years ago by alex

confirmed. It's a QSA/WebKit bug. Working on it now.

comment:10 Changed 11 years ago by alex

Component: GeneralCore
Priority: highesthigh
severity: normalminor

so on a lark, I added a doctype to the test page to throw things into standards mode. Lo-and-behold, case sensitivity in CSS class name matches returned. It seems then, that this is (to some vexing degree) by design in the webkit engine. To that end, I'm not sure this counts as a regression so much as a QSA oddity. I'll still see if there's some fast way to detect the confluence of conditions that lead to problems, but I doubt this is a blocker.

comment:11 Changed 11 years ago by Jared Jurkiewicz

Its a regression, given you can take dojo 1.2.3 and the same app page just works that breaks under dojo 1.3. The difference is that dojo 1.2.3 wasn't using QSA. Usage of new feature under the covers that alters behaviors/breaks existing applications is a regression.

I also disagree with the change of severity. This *will* cause us grief.

comment:12 Changed 11 years ago by Jared Jurkiewicz

And I am willing to (and will) vote against 1.3 releasing until this issue is addressed in some manner.

comment:13 Changed 11 years ago by Nathan Toone

Cc: Nathan Toone added

I agree - 1.3 is completely unworking in safari in cases where 1.2.3 was working. This needs to be resolved before 1.3 is released.

comment:14 Changed 11 years ago by Nathan Toone

Also - if it were some "odd" version of webkit that this were limited to, that would be a different story, but this is manifest in the current version of Chrome as well as the current (default) version of safari in Mac OS.

comment:15 Changed 11 years ago by alex

Resolution: fixed
Status: assignedclosed

(In [16928]) correctness fixes in the DOM branch. Make sure we don't double-count things in "," separated queries and ensure that we don't use QSA in suspect documents on WebKit? (pending webkit fix to case-sensitivity issue). Fixes #8775. !strict

comment:16 Changed 11 years ago by Nathan Toone

This fixes the issue in my appliation. Thanks!

comment:17 Changed 11 years ago by Douglas Hays

Refer to #7869. This change caused a regression in query.html when using Safari 4 public beta (528.16)/winxp:

doh.is(dojo.byId('_foo'), dojo.query('.foo:nth-child(2)')[0]);
[ Error: assertEqual() failed: expected [object HTMLDivElement] but got undefined ]

comment:18 Changed 11 years ago by Douglas Hays

Resolution: fixed
Status: closedreopened

comment:19 Changed 11 years ago by Nathan Toone

I would classify this as a bit more minor (ie don't hold up a beta or a release for it) if it's limited only to safari 4...

comment:20 Changed 11 years ago by alex

note, the change didn't cause the regression (AFAICT). I'm working to see what's gong on w/ nth-child selectors in DOM only-mode.

comment:21 Changed 11 years ago by alex

Resolution: fixed
Status: reopenedclosed

(In [16963]) nth-child selectors in the DOM branch were potentially returning the wrong results since the simpleElementTest would be elided for firstChild nodes that weren't elements. This is fixed by looking for firstElementChild on browsers that can handle it. Fixes #8775. !strict

comment:22 Changed 11 years ago by alex

(In [16964]) removing extraneous parseInt calls. Refs #8775. !strict

comment:23 Changed 11 years ago by alex

(In [16965]) re-enable index cache. Refs #8775. !strict

comment:24 Changed 10 years ago by dante

Resolution: fixed
Status: closedreopened

This appears to be back in FF3.5

tests/_base/query.html is in quirks mode. Apparently, in FF3.5, in quirks mode, dojo.query(".fooBar") matches class="foobar" and class="fooBar", but in standards only matches class="fooBar" ... FF3.1 only matches class="fooBar" in both quirks and standards. Safari4 displays the same behavior as FF3.1

the unit test is only 'supposed' to find one element in the page:

"doh.is(1, dojo.query('.fooBar').length);",

it is locating the span and h3 with fooBar and foobar

comment:25 Changed 10 years ago by Adam Peller

Milestone: 1.31.3.2
Summary: REGRESSION: Camel Cased classname queries fail on Safary 3.2.2 and ChromeREGRESSION: Camel Cased classname queries fail on Safari 3.2.2, Chrome, FF 3.5

comment:26 Changed 10 years ago by dante

I made up a little chart of all the browsers I have handy. Quirks colum is found,expected,foundWithLowerCasequerycall and standards is just found.

			quirks		standard	"correct"
ie8			1,3,1		1,1,1
ie8-comp	1,3,1		1,1,1
ff3.5 xp	3,3,3		1,1,1			*
ff3.1 osx	1,3,1		1,1,1
opera10xp	3,3,3		1,1,1			*
saf4		1,3,1		1,1,1
saf3		1,3,1		1,1,1
webktngly	1,3,1		1,1,1
chrmiumosx	1,3,1		1,1,1

ff3.5 and opera10 both follow the spec of treating classnames insensitive in quirks (defined by the html5 draft), everyone else I tested displayed the "broken" (non-spec) behavior.

Not sure if we should fix for spec or fix for back-compat. My little test is attached.

Changed 10 years ago by dante

Attachment: camelquery.php added

small test to toggle quirksmode and show some numbers.

comment:27 Changed 10 years ago by Adam Peller

Alex, can you take a look at this? We need it to ship 1.3.2.

comment:28 in reply to:  24 Changed 10 years ago by bill

Replying to dante:

the unit test is only 'supposed' to find one element in the page:

"doh.is(1, dojo.query('.fooBar').length);",

it is locating the span and h3 with fooBar and foobar

This really doesn't seem important enough to worry about.

Who would ever have two distinct classnames that only differed in case (like foobar and fooBar), where foobar and fooBar were intended to mean separate things? Surely if that occurred in some code the designer meant for foobar and fooBar to refer to the same thing.

comment:29 Changed 10 years ago by James Burke

I tend to agree with Bill on this, coupled with it being a quirks mode issue, it seems like a hazard of using quirksmode. Maybe the fix is just to convert the base/query.html test to be in standards mode? Or is this causing a serious issue out in real world code?

comment:30 Changed 10 years ago by alex

Resolution: fixed
Status: reopenedclosed

(In [18756]) Fixes #8775 on the 1.3.x branch. I expect this to break again.

!strict

comment:31 Changed 10 years ago by alex

(In [18757]) Refs #8775. I expect this to break again.

!strict

comment:32 Changed 8 years ago by bill

In [28045]:

In standards-mode documents, except for acme, class names are supposed to be case-sensitive. Refs #8775, #14874

comment:33 Changed 7 years ago by bill

In [30111]:

Disable test for attribute case sensitivity in quirks mode, since it's browser dependent and we don't want to bother "fixing" it, whatever that would mean. Avoids test failure on FF. Refs #8775, #14874 on trunk.

comment:34 Changed 7 years ago by bill

In [30112]:

Disable test for attribute case sensitivity in quirks mode, since it's browser dependent and we don't want to bother "fixing" it, whatever that would mean. Avoids test failure on FF. Refs #8775, #14874 on 1.8 branch.

Note: See TracTickets for help on using tickets.