1642922 - Scroll-padding implementation ignores keyboard focus

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Description

[Alastair Campbell]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:76.0) Gecko/20100101 Firefox/76.0

Steps to reproduce:

On a page with a sticky footer and scroll-padding set, e.g. test page: https://alastairc.uk/tests/wcag22-examples/sticky-footer3.html

Press tab to move the keyboard focus down the page.

Actual results:

The keyboard focus moves behind the sticky footer (and header).

Expected results:

The scroll-padding should keep the focus visible in the page, as it does in Chromium.

I'm filing as an enhancement because I couldn't see a place in the scroll-snap spec that defines this behaviour, but I've filed a bug on the spec for that as well:
https://github.com/w3c/csswg-drafts/issues/5066

I consider it a bug, and it would be useful to fix as WCAG 2.2 is placing more emphasis on visible focus styles.

Comment 1

[Peter Magyari (Desktop QA)]

Hi,

I've reproduced the issue on MacOS 10.15 and Windows 10 using Firefox 77.0.1 and Firefox Nightly 79.0a1.

I'm setting a component in order to involve the development team in reviewing this issue.

Thank you for reporting!

Comment 2

[Emilio Cobos Álvarez [:emilio]]

scroll-padding is honored, however it's not honored if the link is already in the viewport, which is clearly wrong.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1642922 - Tweak scroll-padding implementation to also account for visibility. r=hiro

The previous implementation made us think that stuff was visible when in
fact it was not.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f2a4500c96d
Tweak scroll-padding implementation to also account for visibility. r=hiro

Comment 5

[Narcis Beleuzu [:NarcisB]]

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

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Comment on attachment 9156034 [details]
Bug 1642922 - Tweak scroll-padding implementation to also account for visibility. r=hiro

Beta/Release Uplift Approval Request

• User impact if declined: Some links won't be visible when tabbing to them.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): It's not a risk-free patch, though I think it'd be worth having this on ESR... Your call, I'd be ok with it either way.
• String changes made/needed: none

Comment 8

[Julien Cristau [:jcristau]]

Let's have this ride the trains, and reconsider esr78 uplift later.

Comment 9

[Peter Magyari (Desktop QA)]

Verified fixed on Nightly 79.0a1 (20200615092624)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9156034 [details]
Bug 1642922 - Tweak scroll-padding implementation to also account for visibility. r=hiro

This has baked on 79 for about a month now without any reported regressions, was verified on Nightly, and includes automated tests. Approved for 78.1esr.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/f1b4b1f1311f

Comment 12

[Peter Magyari (Desktop QA)]

Verified fixed on Firefox ESR 78.1.0 (20200706204357)