Bug 236782 – [macOS][selectors] :focus-visible matching on button focused via script (after clicking on a different button)

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Manuel Rego Casasnovas]

Bug report from Twitter: https://twitter.com/LegendOfMI/status/1494110668424892416

Test case: https://codepen.io/Blondebeard/pen/JjOOgEE

Clicking on the first button moves the focus to the 2nd button, we shouldn't see an outline on the 2nd button (shouldn't match :focus-visible).

This is a MacOS specific issue, as it doesn't happen on WebKitGTK.

Maybe the fact that buttons are not click focusable on MacOS might be the reason why this is failing, I'd need to debug this on a Mac machine to understand what's going on.

[Manuel Rego Casasnovas]

So this is kind of tricky, there's a test that is testing this behavior explicitly:
https://wpt.fyi/results/css/selectors/focus-visible-script-focus-006.tentative.html
Test title is "CSS Test (Selectors): Script focus after mouse click on a NOT focusable element does match :focus-visible".

The test is marked as tentative, as it reflects current browser behavior, but might be not the intended behavior from the users POV.

There's not a clear spec text about all this, the text in https://drafts.csswg.org/selectors/#the-focus-visible-pseudo is just an example:
> If the previously-focused element indicated focus, and a script causes focus to move elsewhere, the newly focused element should indicate focus.

Like the previous element, the button in Mac, hasn't been actually focused (even when we clicked it), then the browser indicates focus in the next element.

There was an attempt to have a spec text about when to draw the focus indicator, but at the end it was discarded: https://github.com/whatwg/html/pull/6523

So I don't know what should we do regarding this issue. As usual the root issue is that buttons on Safari are not mouse focusable (see bug #229895), otherwise this would be working as expected.

[Radar WebKit Bug Importer]

<rdar://problem/89382543>

[Kai]

This issue is also happening on iOS (15.4) and not limited to macOS: https://twitter.com/schweinepriestr/status/1508748936252698626

In the video, https://codepen.io/Blondebeard/pen/JjOOgEE showing the outline is imho incorrect, or, to put it neutral, different from other browsers.

Additionally the mentioned test
https://wpt.fyi/results/css/selectors/focus-visible-script-focus-006.tentative.html?label=experimental&label=master&aligned=
or rather
http://wpt.live/css/selectors/focus-visible-script-focus-006.tentative.html
doesn't appear to cover this issue fully, as it PASSes but clearly the behavior is different.

If I understand correctly this is not speced behavior, but I would strongly urge WebKit/Safari to match the other browsers.

---

Because imho the current behavior, esp. because of mobile, blocks the usage of `:focus-visible`. Out of nowhere you have all those focus indicators showing up on mobile because previously using .focus() wasn't an issue but is now after switching to `:focus-visible`, so you'd have for example
- wrap `:focus-visible` in a media query only targeting a certain min-with (potentially excluding iPads or small width devices using keyboards)
- not use .focus() on mobile, checking everywhere, potentially rewriting your whole application
- not use `:focus-visible`
All of which are unfavorable options.

[Antti Koivisto]

Seems pretty clear we shouldn't be matching :focus-visible in this case. Maybe button click should also set wasLastFocusByClick state (even though button doesn't get focused) that prevents visible programmatic focus?

[Manuel Rego Casasnovas]

Created attachment 456693 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Manuel Rego Casasnovas]

I've uploaded a workaround to fix this issue.

(In reply to Kai from comment #3)
> This issue is also happening on iOS (15.4) and not limited to macOS:
> https://twitter.com/schweinepriestr/status/1508748936252698626
> 
> In the video, https://codepen.io/Blondebeard/pen/JjOOgEE showing the outline
> is imho incorrect, or, to put it neutral, different from other browsers.
> 
> Additionally the mentioned test
> https://wpt.fyi/results/css/selectors/focus-visible-script-focus-006.
> tentative.html?label=experimental&label=master&aligned=
> or rather
> http://wpt.live/css/selectors/focus-visible-script-focus-006.tentative.html
> doesn't appear to cover this issue fully, as it PASSes but clearly the
> behavior is different.
> 
> If I understand correctly this is not speced behavior, but I would strongly
> urge WebKit/Safari to match the other browsers.

There's actually not a clear spec for all this. But this is what all browsers do when you click on a non focusable element and move focus afterwards. The key here is that buttons are not click focusable on Safari while they're in the other browsers.

(In reply to Antti Koivisto from comment #4)
> Seems pretty clear we shouldn't be matching :focus-visible in this case.
> Maybe button click should also set wasLastFocusByClick state (even though
> button doesn't get focused) that prevents visible programmatic focus?

It seems clear we don't want this behavior for :focus-visible, so I provided a workaround.

Still I'm not sure if the current behavior of buttons on Safari is also what we want, as other interop issues appear due to that (e.g. you click a button, you later use spacebar to click it again, and that doesn't happen as it hasn't been focused).

[Antti Koivisto]

Comment on attachment 456693 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456693&action=review

> Source/WebCore/page/EventHandler.cpp:2798
> +#if (!PLATFORM(GTK) && !PLATFORM(WPE))

Maybe better to say #if PLATFORM(COCOA) if we assume other platforms don't need this behavior.

> Source/WebCore/page/EventHandler.cpp:2800
> +    // Form control elements are not mouse focusable on some platforms (see HTMLFormControlElement::isMouseFocusable())

Could just say "Apple platforms", no need to be coy.

[Manuel Rego Casasnovas]

(In reply to Antti Koivisto from comment #8)
> Comment on attachment 456693 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=456693&action=review
> 
> > Source/WebCore/page/EventHandler.cpp:2798
> > +#if (!PLATFORM(GTK) && !PLATFORM(WPE))
> 
> Maybe better to say #if PLATFORM(COCOA) if we assume other platforms don't
> need this behavior.

WIN port also needs this, because of the #if in HTMLFormControlElement::isMouseFocusable().
See bug #221036.

That's why I preferred to put the same condition than in the other method.

> > Source/WebCore/page/EventHandler.cpp:2800
> > +    // Form control elements are not mouse focusable on some platforms (see HTMLFormControlElement::isMouseFocusable())
> 
> Could just say "Apple platforms", no need to be coy.

As mentioned before it's not just Apple platforms. Win port (probably not on purpose) does the same.

[Antti Koivisto]

> Still I'm not sure if the current behavior of buttons on Safari is also what
> we want, as other interop issues appear due to that (e.g. you click a
> button, you later use spacebar to click it again, and that doesn't happen as
> it hasn't been focused).

The right place to discuss that is bug 22261. https://bugs.webkit.org/show_bug.cgi?id=22261#c73 gives the full rationale for the current behavior.

[Manuel Rego Casasnovas]

(In reply to Antti Koivisto from comment #10)
> > Still I'm not sure if the current behavior of buttons on Safari is also what
> > we want, as other interop issues appear due to that (e.g. you click a
> > button, you later use spacebar to click it again, and that doesn't happen as
> > it hasn't been focused).
> 
> The right place to discuss that is bug 22261.
> https://bugs.webkit.org/show_bug.cgi?id=22261#c73 gives the full rationale
> for the current behavior.

Yeah I know the rationale and I've followed that discussion.

Anyway that bug is now closed and the discussion has moved to https://bugs.webkit.org/show_bug.cgi?id=229895 these days.

[EWS]

Committed r292400 (249263@main): <https://commits.webkit.org/249263@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456693 [details].

[Sam Sneddon [:gsnedders]]

Just to let everyone know, the fix for this has shipped in Safari 15.5.