1387934 - stylo: inDOMUtils::SelectorMatchesElement should not match with bogus pseudo

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+11]

layout/inspector/tests/test_selectormatcheselement.html fails because of this.

It returns true for
> utils.selectorMatchesElement(element, elementRule, 0, ":bogus")
where the bogus pseudo shouldn't match anything actually.

This should be an easy one to fix. Just do a special check before https://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/layout/style/ServoStyleRule.cpp#276-291 should be enough.

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Bug 1387934 - stylo: inDOMUtils::SelectorMatchesElement should not match with bogus pseudo.

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

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8896322 [details]
Bug 1387934 - stylo: inDOMUtils::SelectorMatchesElement should not match with bogus pseudo.

https://reviewboard.mozilla.org/r/167574/#review173034

::: layout/style/ServoStyleRule.cpp:288
(Diff revision 1)
> +  *aMatches = (aPseudo.IsEmpty() || pseudoType != CSSPseudoElementType::NotPseudo)
> +    ? Servo_StyleRule_SelectorMatchesElement(mRawRule,
> -                                                     aElement,
> +                                             aElement,
> -                                                     aSelectorIndex,
> +                                             aSelectorIndex,
> -                                                     pseudoType);
> +                                             pseudoType)
> +    : false;

Doesn't seem to be very readable... Could you just do an early return for that case?

Actually, I guess we can just skip `NS_Atomize` and `GetPseudoType` if `aPseudo` is empty, which can make the function slightly faster and should also make the code clearer.

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8896322 [details]
Bug 1387934 - stylo: inDOMUtils::SelectorMatchesElement should not match with bogus pseudo.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/167574/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cbcc0b81f21
stylo: inDOMUtils::SelectorMatchesElement should not match with bogus pseudo. r=xidorn

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5cbcc0b81f21