1856285 - Hover/active quirk doesn't account properly for `:is()` / `:where()`

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Sebastian Zartner [:sebo]]

The :is() pseudo-class currently doesn't match form elements when followed by a :hover or :active pseudo-class.
I didn't try all combinations, though it does work for other pseudo-classes like :focus, :focus-within or :first-child.
And this issue also seems to be restricted to form fields. Other elements like anchors work fine. Though again, I didn't test all of them.

STR:

1. Go to data:text/html,<button>Button</button><input value="Input"><a href="">Link</a><style>:is(button,input,a){background-color:yellow;}:is(button,input,a):hover{background-color:lime;}:is(button,input):focus{background-color:skyblue;}</style>
2. Hover the button or the input field

Actual:
The fields still have a yellow background on hover (bug), while the anchor gets a lime background on hover (ok).

Expected:
The fields should get a lime background on hover like the anchor.

I tested this with Firefox 118.0, Nightly 120.0a1 (2023-10-01) and also Firefox 100.0 and 78.0 (where :is() got released) and they all have the same issue. So it is not a regression.

Sebastian

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Test-case from comment 0

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Test-case from comment 0 (standards mode)

Comment 3

[Emilio Cobos Álvarez [:emilio]]

I guess this is https://quirks.spec.whatwg.org/#the-active-and-hover-quirk.

This code isn't recursing into nested selectors etc.

We should just add Is / Where / Has and other pseudo-classes that aren't a NonTSPseudoClass to that match statement (might be easier to invert the statement actually, since any new thing we probably don't want to apply the quirk to...).

We should extend test_hover_quirk for this.

Comment 4

[Niels]

Hi, I'd like to start working on fixing this bug. This would be my first bug for the mozilla codebase.
If I understand correctly, Is / Where / Has and other pseudo-classes that aren't a NonTSPseudoClass should be added to the match statement, returning true in those cases. Or it should be inverted, returning true on those cases, pseudo_class.is_active_or_hover() for NonTSPseudoClass and false for the rest.

Comment 5

[Niels]

Attachment: Bug 1856285 Invert hover_and_active_quirk_applies match statement and extend test_hover_quirk. r=emilio

Comment 6

[Niels]

I added Is / Where / Has and any other pseudo classes or elements i could find in Component.
I took a look at test_hover_quirk and I wouldn't know how to go about extending the actual test. It seems to want to run the tests from a SimpleTest file that doesn't exist.
I also looked at trying to invert the statement, where it would just return false on any case that's not a NonTSPseudoClass but I'm not certain about doing it this way since I'm not sure if there were ever any cases where the current statement ends in the _ => true.

Comment 7

[Niels]

Attachment: Bug 1856285 Invert hover_and_active_quirk_applies match statement and extend test_hover_quir. r=emilio

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43344c78238c
Invert hover_and_active_quirk_applies match statement and extend test_hover_quirk. r=emilio

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/43344c78238c

Comment 10

[Ina Popescu, Desktop QA]

I've reproduced this issue using Nightly 125.0a1 on Windows10 x64 following the STR from Comment 0.
Verified as fixed in the latest Firefox 126.0b3 version on Windows10 x64, macOS 13 and Ubuntu 22.04, as the issue no longer persists.