1363531 - stylo: Support case insensitive HTML attributes

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P1)

Description

[Manish Goregaokar [:manishearth]]

A bunch of HTML attributes are case insensitive wrt style, which means that in an HTML document, the selector `[dir=rtl]` must match `<span dir="RTL">` too.


https://searchfox.org/mozilla-central/source/layout/style/nsCSSParser.cpp#5853

When parsing selectors, Gecko will check if the selector is an attr selector for one of these, and store an extra bit alongside the selector that it is case sensitive. It will then check this[1][2] whilst matching attributes and matches appropriately.

We need something similar. AFAICT this will require rust-selectors changes; while we could make SelectorImpl::Attribute into an (Atom, bool), this kind of gets awkward since the attributes we match against will have to get a dummy bool (computing the bool requires a slew of string comparisons and is expensive; so we can't do this during traversal); and it makes == stop being reflexive.

I guess the simple thing to do here would be to store a bool in attr selectors, have SelectorImpl have a function for determining if an attr is case insensitive, and pass down an extra bool (which is this bool + whether or not the doc is HTML) to the match_attr_foo functions on selectors::MatchAttr.


Thoughts?

This unblocks a lot of tests using `dir`.


 [1]: https://searchfox.org/mozilla-central/source/layout/style/nsCSSRuleProcessor.cpp#1400
 [2]: https://searchfox.org/mozilla-central/source/layout/style/nsCSSRuleProcessor.cpp#2236

Comment 1

[Anthony Ramine [:nox]]

https://html.spec.whatwg.org/multipage/#case-sensitivity-of-selectors

This could probably be filed as a E-mentored issue on servo/servo.

Comment 2

[Simon Sapin (:SimonSapin)]

I think we should definitely not smuggle an extra bool through SelectorImpl::Attribute. We can add it to AttrSelector (possibly packed with NamespaceConstraint if it helps avoid growing size_of::<selector::Component>()). We moved rust-selectors in-tree to make this kind of change easier :)

Comment 3

[Bobby Holley (:bholley)]

Don't we already have CaseSensitivity on AttrEqual?

Also filed bug 1364148 about making Component smaller.

Comment 4

[Bobby Holley (:bholley)]

Also, does the case-sensitivity here apply only to AttrEqual, or to all the attribute selectors?

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Also, does the case-sensitivity here apply only to AttrEqual, or to all the attribute selectors?

All of them.  Looks like Servo generally screws this up for things other than AttrEqual (e.g. it supports [foo="bar"i] but not [foo^="bar"i]).  I filed bug 1364162 on that.

We should probably have tests in test_selectors.html for this if we don't have them already.

Note that what Gecko has right now is a case-sensitivity tristate, with three values:

1)  Case-sensitive. ([foo="bar"])
2)  Case-insensitive  ([foo="bar"i])
3)  Case-insensitive only if the element is HTML ([dir="RTL"]).

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

Servo side: servo/servo#15006

Comment 7

[Simon Sapin (:SimonSapin)]

[1] says that the value of `type` attributes (among others) is case-insensitive, except in the bit of user-agent stylesheet [2] that sets list-style-type based on the type attribute of ul, ol, and li. However I can’t find these rules in layout/style/res/*.css. Are they implemented in C++ without selector matching? If so, we don’t need to worry about this exception to the exception.

[1] https://html.spec.whatwg.org/multipage/scripting.html#case-sensitivity-of-selectors
[2] https://html.spec.whatwg.org/multipage/rendering.html#attribute-selector-case-sensitive

Comment 8

[Xidorn Quan [:xidorn] UTC+11]

Those list-style-type mappings are implemented via attribute mapping. We are not using ua sheet for that.

Comment 9

[Simon Sapin (:SimonSapin)]

I have a patch that goes on top of https://github.com/servo/servo/pull/16915. I’ll open a new PR after that one lands.

(Canceling needinfo since Xidorn responded.)

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Those list-style-type mappings are implemented via attribute mapping

Indeed.  Note that in the spec these are defined as presentational hints, not UA-level rules.  For Gecko that means implementation as mapped attributes in C++, for the moment.  So no need to worry about these for selector matching purposes.

Comment 11

[Simon Sapin (:SimonSapin)]

Fixed in https://github.com/servo/servo/pull/16915. It uses the same mechanism as [foo=bar i], so bug 1364162 applies.