This is a followup to bug 356936. The list of case sensitive HTML attributes for purposes of CSS attribute selector matching should probably instead be a list of case-insensitive attributes, so that any new attributes added are case sensitive by default. See bug 356936 for more details. Also, while we're there, we should probably also convert to |static const char caseSensitiveHTMLAttribute[][13]| for the reduction in shared library relocations. (The space tradeoff is probably about even -- it makes all the strings take up 13 characters, but saves a pointer for each one.)

The list of case-insensitive attributes is as follows: lang, dir, http-equiv, text, link, vlink, alink, compact, align, frame, rules, valign, scope, axis, nowrap, hreflang, rel, rev, charset, codetype, declare, valuetype, shape, nohref, media, bgcolor, clear, color, face, noshade, noresize, scrolling, target, method, enctype, accept-charset, accept, checked, multiple, selected, disabled, readonly, language, defer, type

The default behavoir is case-sensitive. Only if the parser is set to case-insensitive AND the attribute is present in the list it is treated in a case-insensitive way.

Comment on attachment 243195 [details] [diff] [review] Patch against trunk Oops, I lost this since there was no review request. Issues I need to look at: * I think we need to be more careful about namespace ID == None now -- we need to make sure it really only applies to HTML. * we should probably use static const char[][15] = { ... }

Yes, please don't treat all namespace-less elements as HTML elements. Call nsINode::IsNodeOfType(nsINode::eHTML) instead. (though i know in some parts of the selector matching code we artificially sets the namespace to kNameSpaceID_XHTML for such nodes, not sure if that happens here though). Also, should this list apply to xhtml? Or is that always case sensitive?

So, we're already checking mCaseSensitive, so things aren't so bad -- although we really shouldn't be doing case sensitivity at parse time, but that's another bug. We really want to check only kNameSpaceID_None, since we're looking at the namespace on the *attribute*.

Comment on attachment 243195 [details] [diff] [review] Patch against trunk Which means, in other words, that this patch is fine. I'm just going to change this: >+ if (mCaseSensitive == PR_FALSE) { >+ if (nameSpaceID == kNameSpaceID_None || >+ nameSpaceID == kNameSpaceID_XHTML) { to use &&, which will means that there will be fewer indentation changes as well. I'll also file a followup bug on removing the check for kNameSpaceID_XHTML there. r+sr=dbaron with that change -- I'll check it in shortly

Patch checked in to trunk. Thanks for the patch, and sorry for the huge delay. Bug 282328 covers deferring making case-sensitivity decisions (at least, that's one of the options for it). I filed bug 387615 as a followup on removing the kNameSpaceID_XHTML check.

(In reply to comment #3) > * we should probably use static const char[][15] = { ... } Surely 16 would be more efficient ;-)

I just added mozilla/layout/style/test/test_bug357614.html .