Closed Bug 1642922 Opened 1 month ago Closed 20 days ago

Scroll-padding implementation ignores keyboard focus

Categories

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

Unspecified
All
defect

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox-esr78 --- affected
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- verified

People

(Reporter: ac, Assigned: emilio)

Details

Attachments

(1 file)

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.

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!

Status: UNCONFIRMED → NEW
Type: enhancement → defect
Component: Untriaged → DOM: UI Events & Focus Handling
Ever confirmed: true
OS: Unspecified → All
Product: Firefox → Core
Version: 76 Branch → Trunk

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

Flags: needinfo?(emilio)
Assignee: nobody → emilio
Flags: needinfo?(emilio)

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

Severity: -- → S3
Priority: -- → P2
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5f2a4500c96d
Tweak scroll-padding implementation to also account for visibility. r=hiro
Status: NEW → RESOLVED
Closed: 20 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79

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.

Flags: needinfo?(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
Flags: needinfo?(emilio)
Attachment #9156034 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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

Attachment #9156034 - Flags: approval-mozilla-esr78?
Attachment #9156034 - Flags: approval-mozilla-beta?
Attachment #9156034 - Flags: approval-mozilla-beta-

Verified fixed on Nightly 79.0a1 (20200615092624)

QA Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.