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

RESOLVED FIXED in Firefox 55

Status

()

Core
CSS Parsing and Computation
P2
normal
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: bz, Assigned: manishearth)

Tracking

(Blocks: 2 bugs)

53 Branch
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

3 months ago
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.
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.
Priority: -- → P2
(Assignee)

Comment 2

2 months ago
Affects WPT /selectors/attribute-selectors/attribute-case/semantics.html
Blocks: 1370466
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 months ago
Assignee: nobody → manishearth
Status: NEW → ASSIGNED
(Assignee)

Comment 5

2 months ago
mozreview-review
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

2 months ago
mozreview-review
Comment on attachment 8875918 [details]
Bug 1364162 - Part 1: stylo: Remove TElement::attr_equals ;

https://reviewboard.mozilla.org/r/147316/#review151570
Attachment #8875918 - Flags: review?(simon.sapin) → review+

Comment 7

2 months ago
mozreview-review
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.
Attachment #8875919 - Flags: review?(simon.sapin) → review+
(Assignee)

Comment 8

2 months ago
https://github.com/servo/servo/pull/17247

Comment 9

2 months ago
Pushed by manishearth@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/45dfceacb9e8
Part 2: stylo: Make all attribute selectors respect case insensitivity; r=SimonSapin

Comment 10

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/45dfceacb9e8
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.