Bug 15470 – Make attr selectors case-sensitive for case-sensitive HTML attrs (Acid3 bug)

• Component: CSS
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

Make attr selectors case-sensitive for case-sensitive HTML attrs

currently we fail 18 tests in this suite:
http://www.css3.info/selectors-test/

because of this "bug".  The justification is that the CSS2 spec says that case sensitivity for values (other than element names) is "language dependent":

http://www.w3.org/TR/css3-selectors/#attribute-selectors

And HTML has certain attributes which are case sensitive and others which are not:
http://www.w3.org/TR/REC-html40/types.html#h-6.1

Strangely enough, both Opera and FireFox pass different subsets of these 18 tests.  They seem to be related to implementation details in both browsers.  Given that neither of them fail all 18 like we do (but rather seem to attempt to support this strangeness), I think this patch is correct.

[Eric Seidel (no email)]

Created attachment 16636 [details]
Fix (test updates forthcoming)

[Maciej Stachowiak]

I think it's worth testing this in both quirks mode and standards mode in other browsers (especially Firefox and IE).

[Darin Adler]

Comment on attachment 16636 [details]
Fix (test updates forthcoming)

htmlAttributeHasCaseSensitiveValue should probably use a set instead of 18 separate calls. Can first check that there's no namespace, then use a set of AtomicString for the attr names.

+            if (caseSensitive && sel->m_value != value)
+                return false;
+            else if (!caseSensitive && !equalIgnoringCase(sel->m_value, value))
                 return false;

No need to else after return.

I'd write it with ? : instead:

    if (!(caseSensitive ? sel->m_value == value : equalIgnoringCase(sel->m_value, value)))
        return false;

I think it's a bit unfortunate that we're going to be computing the caseSensitive boolean all the time. Does every case in the switch statement use it?

review- for the set issue

[Eric Seidel (no email)]

*** Bug 16703 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

it is unlikely I'll get to this patch again anytime soon.

[Eric Seidel (no email)]

Created attachment 18300 [details]
Case sensitivity fix, including testcase

 LayoutTests/ChangeLog                              |   11 ++
 .../css/html-attr-case-sensitivity-expected.txt    |  129 ++++++++++++++
 .../fast/css/html-attr-case-sensitivity.html       |   13 ++
 .../css/resources/html-attr-case-sensitivity.js    |  176 ++++++++++++++++++++
 WebCore/ChangeLog                                  |   15 ++
 WebCore/css/CSSStyleSelector.cpp                   |   84 +++++++++-
 6 files changed, 420 insertions(+), 8 deletions(-)

[Darin Adler]

Comment on attachment 18300 [details]
Case sensitivity fix, including testcase

+// test a non-existant attr

This should say "nonexistent", not "non-existant".

r=me

[Eric Seidel (no email)]

Yay!  we now pass the 18 css3 selector tests related to this bug!
r29212

[Robert Blaut]

Please close a bug 11389 as a duplication of this bug.

[Robert Blaut]

Also add this bug to meta bug 11390.

[Alexey Proskuryakov]

*** Bug 11389 has been marked as a duplicate of this bug. ***