356936 - List of case-sensitive attributes for the CSS Attribute Selector is incomplete

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Niels Leenheer (HTML5test)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7

The list of attributes that should be treated in a case-sensitive way is incomplete. The current list only includes some basic attributes:
http://lxr.mozilla.org/seamonkey/source/layout/style/nsCSSParser.cpp#2365

The list should include all of the following attributes:
abbr, action, alt, archive, background, cite, class, classid, code, codebase, content, data, datetime, for, headers, href, id, label, longdesc, name, object, onblur, onchange, onclick, ondblclick, onfocus, onkeydown, onkeypress, onkeyup, onload, onmousedown, onmousemove, onmouseout, onmouseover, onmouseup, onreset, onselect, onsubmit, onunload, profile, prompt, scheme, src, standby, summary, title, usemap, value

More discussion about this bug can be found here:
http://rakaz.nl/item/css_selector_bugs_case_sensitivity
http://bugs.kde.org/show_bug.cgi?id=135505

Reproducible: Always

Comment 1

[Niels Leenheer (HTML5test)]

Attachment: Patch against trunk

Comment 2

[Niels Leenheer (HTML5test)]

Attachment: Patch against MOZILLA_1_8_BRANCH

Comment 3

[Joost de Valk]

I can confirm that the bug is there.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 242496 [details] [diff] [review]
Patch against trunk

A more useful representation of this patch is:
>               "action",
4a6
>               "background",
6a9,13
>               "classid",
>               "code",
>               "codebase",
>               "content",
>               "data",
7a15,16
>               "for",
>               "headers",
8a18
>               "id",
9a20
>               "longdesc",
10a22,39
>               "object",
>               "onblur",
>               "onchange",
>               "ondblclick",
>               "onfocus",
>               "onkeydown",
>               "onkeypress",
>               "onkeyup",
>               "onload",
>               "onmousedown",
>               "onmousemove",
>               "onmouseout",
>               "onmouseup",
>               "onreset",
>               "onselect",
>               "onsubmit",
>               "onunload",
>               "profile",
11a41,42
>               "scheme",
>               "src",
14a46,47
>               "usemap",
>               "value",


We should make these just one-per-line so that diffs are readable.  And probably also convert to |static const char caseSensitiveHTMLAttribute[][13]| for the reduction in relocations.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

My other concern here is how we should be treating unknown attributes.  It seems more sensible to treat them case-sensitively -- if they're case insensitive that's easier to fix later.

Hixie, any thoughts, either on the list or on the question of handling unknown attributes?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The list looks fine to me after a quick skim, although I'm not sure if we should be adding code for handling of attributes that we'll probably never support (scheme?).

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 242499 [details] [diff] [review]
Patch against MOZILLA_1_8_BRANCH

These two patches are exactly the same, so no need for both.  We also don't generally consider patches for branches until after they're on the trunk.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

For lack of response, I checked in the patch with the change to one string per line, and with the comment changed to refer to the always-current version of HTML4 rather than the old REC-html40.

I filed bug 357614 as followup for the other issues.

Thanks for figuring this out and submitting the patch.

Comment 9

[Hixie (not reading bugmail)]

dbaron: I agree with everything you've said here.

Comment 10

[Hixie (not reading bugmail)]

HTML5 has now been updated to require what this patch eventually evolved to be.