1364162 - stylo: Need to support case-insensitive matching for attribute selectors other than equality

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

This needs rust-selectors and other servo-side changes.  Right now in servo [foo="bar"i] works but [foo^="bar"i] does not.  Same for the other attribute matching operators.

Comment 1

[Simon Sapin (:SimonSapin)]

https://github.com/servo/servo/pull/16915 does the parsing for this. snapshot.rs and wrapper.rs in servo/components/style/gecko now each have a attr_matches method where a ignore_case boolean is passed to Gecko_SnapshotAttrEquals and Gecko_AttrEquals but ignored in other cases.

What’s needed now is to add pass these booleans through FFI, and implement matching accordingly.

Comment 2

[Manish Goregaokar [:manishearth]]

Affects WPT /selectors/attribute-selectors/attribute-case/semantics.html

Comment 3

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1364162 - Part 1: stylo: Remove TElement::attr_equals ;

Review commit: https://reviewboard.mozilla.org/r/147316/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/147316/

Comment 4

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1364162 - Part 2: stylo: Make all attribute selectors respect case insensitivity;

Review commit: https://reviewboard.mozilla.org/r/147318/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/147318/

Comment 5

[Manish Goregaokar [:manishearth]]

Comment on attachment 8875919 [details]
Bug 1364162 - Part 2: stylo: Make all attribute selectors respect case insensitivity;

https://reviewboard.mozilla.org/r/147318/#review151564

::: layout/style/ServoBindings.cpp:924
(Diff revision 1)
> -               nsIAtom* aStr)
> +               nsIAtom* aStr, bool aIgnoreCase)
>  {
> -  auto match = [aStr](const nsAttrValue* aValue) {
> +  auto match = [aStr, aIgnoreCase](const nsAttrValue* aValue) {
>      nsAutoString str;
>      aValue->ToString(str);
> -    const nsDefaultStringComparator c;
> +    WITH_COMPARATOR(aIgnoreCase, c,

Using macros to wrap the closures would probably be more efficient (since it won't have to check aIgnoreCase each time), but the code is neater this way. Not sure which I should do.

Comment 6

[Simon Sapin (:SimonSapin)]

Comment on attachment 8875918 [details]
Bug 1364162 - Part 1: stylo: Remove TElement::attr_equals ;

https://reviewboard.mozilla.org/r/147316/#review151570

Comment 7

[Simon Sapin (:SimonSapin)]

Comment on attachment 8875919 [details]
Bug 1364162 - Part 2: stylo: Make all attribute selectors respect case insensitivity;

https://reviewboard.mozilla.org/r/147318/#review151576

Looks good to me. I *think* the only case where the closures would be called more than once is with `[*|foo=bar]` wildcard namespaces, which are probably rare.

Comment 8

[Manish Goregaokar [:manishearth]]

https://github.com/servo/servo/pull/17247

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45dfceacb9e8
Part 2: stylo: Make all attribute selectors respect case insensitivity; r=SimonSapin

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/45dfceacb9e8