282328 - attributes are case (in)senstive in xul.css and userChrome.css

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Alfred Kayser]

When setting the folloing in userChrome.css:

@namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
/* set default namespace to XUL */

toolbarbutton[updateCount="0"] { display: box !important; background-color: red
!important; }

One should expect the 'update button' to be shown with a red background in the
toolbar of Firefox. However, inspecting with the DOM Inspector, one can see that
the 'updateCount' gets converted to 'updatecount', so that it doesn't match
anymore with the actual updateCount attribute.

I don't know if this is intentional behaviour, but in this way one cannot
overrule the styling with matchers such as [updateCount="0"]. There is not even
a simple work around for this.

Note, this was found when trying to hide the 'software update' toolbar button in
themes which make them display always (also when there are no updates) from
userChrome.css

Comment 1

[Alfred Kayser]

Normal behaviour for style sheets seems to be case insensitive parsing (and
matching?). However, the updateCount mentioned above (and as set in Firefox
internally) doesn't match with the by the parser lowercased 'updatecount'
attribute. 

If style sheet attributes are indeed case insensitive, then also the attributes
as set in Firefox 'software update' code:
(http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/update/content/updates.xml#92)
need to be lowercased before the 'insensitive matching' will work.

Comment 2

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

Ugh.  The CSS parser has case sensitive and case-insensitive modes, since they
should be case-insensitive for HTML.  However, it's possible that a stylesheet
could be used for both, so we really should probably not do any case conversion
in the parser, but instead do the conversion later on (which might be slower).

Comment 3

[Boris Zbarsky [:bzbarsky]]

Hmm... yeah, I bet this fails for html.css on XHTML pages that use uppercase
attributes too (which is a much more serious issue, since html.css needs to work
for both HTML and XHTML, while we could change userChrome.css to be
case-sensitive always with no problems).

Even worse than attributes (which are not matched on all that often, really) are
tagnames.  Those are matched on a lot.  Worse, they're stored in atom form, and
matched as tags.  Changing this would probably hurt.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

How could the same stylesheet be used in both html and xml environments?
Couldn't we reparse the stylesheet so that we get it in the 'right mode' when
used in a html vs. xml document?

The only case I can think of where you could argue that we need to support both
at once is in a document that mixes xml and html. However such documents will
have more issues and are recommended against in the DOM spec (and the DOM is the
only way to produce such documents).

In fact, I would argue that we should do more casefolding and casefold
classnames too. We do a lot of classname matches and they are pretty slow and
would be even worse if we were to support full unicode caseinsetivity.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> How could the same stylesheet be used in both html and xml environments?

ua.css (which imports xul.css, html.css, etc) is effectively a singleton; while
we clone it for each particular document viewer, that clones the already-parsed
data structures.

I think that's the only way; XUL stylesheets are only shared amongst XUL
documents....

> Couldn't we reparse the stylesheet so that we get it in the 'right mode' when
> used in a html vs. xml document?

I really don't think we want to reparse all of ua.css one every pageload. 
Perhaps we could keep an "html" version and a "non-HTML" version in memory at
all times (and the same for all user stylesheets) and stick the right one in the
style set.  Seems wasteful of memory, though.

> and casefold classnames too.

In HTML, the value of the "class" attribute is case-sensitive, per spec.  We
only do case-insensitive classname matching in quirks mode.

Comment 6

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

How strong is the need to do the case-folding of the stylesheet at parse-time? 
Are case-insensitive comparisons that expensive?  (And are there other ways to
make them cheaper, like lowercasing when constructing the RuleHash?)

Comment 7

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

Hrm, never mind, I guess the atoms issue probably does make a big difference. 
So we should probably just parse two versions.

Comment 8

[Hixie (not reading bugmail)]

(you can end up with mixed XML/HTML content using XBL, too.)

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Yeah, i think two versions are the way to go. The XBL issue might be a problem
though, I need to think more about that.

Comment 10

[Boris Zbarsky [:bzbarsky]]

The XBL thing is more or less broken no matter what, given that in quirks mode
we rather have to parse differently, so the same stylesheet and the same XBL
will behave differently depending on whether the host document is in quirks mode
or not.

Comment 11

[Hixie (not reading bugmail)]

Yeah I wouldn't worry about XBL really.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

One saving grace is the fact that all attr and element names in html.css are and
most likly always will be all lowercase. So whether we casefold or not doesn't
matter.

The only time it'll matter is for userChrome.css and userContent.css where the
user might use attr/element names that contain uppercase characters. And
userChrome.css (and xul.css) should only be requested in casesensitive docs so
if we do the parsing and caching lazily we'll only get one copy of them. So in
theory only userContent.css is the only one we need to keep around two copies of.

Though of course it might be simpler to implement keeping two copies of html.css
too.

Comment 13

[Benjamin Smedberg]

I need to add a rule:

textbox[type="autocomplete"][searchSessions] to xul.css. This is currently impossible because xul.css is always normalized to lowercase. Is there a simple hack we can do to force xul.css to be parsed in case-sensitive mode? 

@-moz-case-sensitive(true);

I'm willing to do some coding on this if somebody can point me to the expedient solution (fixing xul.css, I really don't care about userChrome).

Comment 14

[Boris Zbarsky [:bzbarsky]]

I'm really thinking we should just consider splitting up ua.css a bit, using nsLayoutStylesheetCache more, and having two versions of html.css (and probably of forms.css) in it.  Note that we could get away with just a case-insensitive version of quirk.css.  And that if we did things this way we might be able to simplify the enabling/disabling of quirk.css (just add/remove it in the style set) and possibly even eliminate the cloning of the UA sheet (which we currently only need because of quirk.css, I believe).

Comment 15

[Benjamin Smedberg]

> I'm really thinking we should just consider splitting up ua.css a bit, using
> nsLayoutStylesheetCache more, and having two versions of html.css (and probably
> of forms.css) in it.  Note that we could get away with just a case-insensitive

Is it possible for a single stylesheet to be parsed in both modes and store both the lowercase and the mixed-case atom in the style rules? Then the decision could be made at the time the rules are resolved (based on the document type) rather than at parsing time.

> version of quirk.css.  And that if we did things this way we might be able to
> simplify the enabling/disabling of quirk.css (just add/remove it in the style
> set) and possibly even eliminate the cloning of the UA sheet (which we
> currently only need because of quirk.css, I believe).

Is this something I can help with? I only about 45% understood the sentence above.

Comment 16

[Boris Zbarsky [:bzbarsky]]

> Is it possible for a single stylesheet to be parsed in both modes and store
> both the lowercase and the mixed-case atom in the style rules?

Hmm... Possibly...  That might take a lot more hacking of more different pieces, though...

> Is this something I can help with?

Almost certainly.

> I only about 45% understood the sentence above.

Which parts were confusing?  I'm happy to explain here, on IRC, or over e-mail.

Comment 18

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

This should now be fixed by bug 507762.