Closed Bug 1597993 Opened 11 months ago Closed 9 months ago

Keyboard navigation gets trapped in the Login Item

Categories

(Firefox :: about:logins, defect, P1)

Desktop
All
defect

Tracking

()

VERIFIED FIXED
Firefox 74
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 --- wontfix
firefox72 --- wontfix
firefox73 --- wontfix
firefox74 --- verified

People

(Reporter: srosu, Assigned: abruere)

References

(Depends on 1 open bug, Regression)

Details

(Keywords: access, regression, Whiteboard: [access-p2])

Attachments

(3 files)

Attached video Tabbing.mp4

[Affected versions]:

  • Latest Firefox Nightly 72.0a1 (Build ID: 20191120094758) (64-bit)
  • Firefox Beta 71.0b11 (Build ID: 20191118154140) (64-bit)

[Affected Platforms]:

  • Mac 10.14
  • Windows 10 x64
  • Arch Linux x64

[Prerequisites]:

  • Have a Firefox profile with at least one saved login.

[Steps to reproduce]:

  1. Open the Firefox browser with the profile from prerequisites.
  2. Navigate to the “about:logins” page.
  3. Press the “Tab” key until all elements from the page are focused.
  4. Repeat the previous step and observe the behavior.

[Expected result]:

  • All elements from the page are focused.

[Actual result]:

  • The focus remains trapped in the Login Item mode.

[Regression]

  • The issue is not reproducible with older Nightly 71.0a1 builds. Considering this, using mozregression tool I have found the issue that broke this in the latest Nightly build. Here are the results:
  • Pushlog: https://tinyurl.com/y2benyvj
  • Last good revision: 263936aecc1dcd15f9dadbf4e41f7e7eebad29a6
  • First bad revision: ab46062892d418b37dbfa35fc3a61db5f7c9de59

[Additional Notes]:

  • Attached a screen recording with the issue.

[Tracking Requested - why for this release]: a11y regression

Bug 1586601 seems like the only likely culprit in that regression range but it's not immediately obvious what is going on. Maybe the markup change made some CSS apply differently.

Keywords: access
Priority: -- → P1
Regressed by: 1586601

(Interesting that changing direction and tabbing back breaks out of this)

We have shipped our last beta for 71, since this is a tracked P1,, I am setting the status as fix-optional for 71 in case there is a safe uplift possible in a potential dot release as a ride-along.

(Noting that the offending rule is the overflow: auto on :host in login-item.css.)

I filed a new bug 1604140 after digging into this one and successfully reproducing it with a simple component. If this new bug is confirmed, this would mean that current bug is not a regression due to Bug 1586601, but a long-standing shadow DOM-related bug it revealed.

@ddurst thank you for your comments that helped me to reproduce this focus trap.

Still willing to contribute to any effort if needed.

Thank you very much!

(In reply to Andy Bruère from comment #5)

If this new bug is confirmed, this would mean that current bug is not a regression due to Bug 1586601, but a long-standing shadow DOM-related bug it revealed.

It can be both a regression due to bug 1586601 and a long-standing DOM issue :) Bug 1586601 changed about:logins to hit this issue so it's still a regression from the about:logins user standpoint. If we can achieve the same end-result while not hitting that bug then it's an option to consider, OR we can wait for your bug 1604140 to get fixed.

Depends on: 1604140

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #6)

Thank you very much!

(In reply to Andy Bruère from comment #5)

If this new bug is confirmed, this would mean that current bug is not a regression due to Bug 1586601, but a long-standing shadow DOM-related bug it revealed.

It can be both a regression due to bug 1586601 and a long-standing DOM issue :) Bug 1586601 changed about:logins to hit this issue so it's still a regression from the about:logins user standpoint. If we can achieve the same end-result while not hitting that bug then it's an option to consider, OR we can wait for your bug 1604140 to get fixed.

Indeed, thank you for taking the time to correct my statement!

I will keep digging into some existing bugs and offer my help without creating extra ones ;)

I had a look at the existing tests: I could help adding a few focus-related assertions (some of which would obviously fail for now) in https://searchfox.org/mozilla-central/source/browser/components/aboutlogins/tests/chrome/test_login_item.html similar to keyboard navigation tests for test_login_list, to prevent similar regressions.

Whiteboard: access-p2
Whiteboard: access-p2 → [access-p2]

Did bug 1607223 affect the behavior here at all? Note that even if it did help here, I'm not sure that's something we'd want to uplift to Beta for Fx73. If there's any sort of workaround we can land in the mean time, I think it would be worth doing so to fix this usability regression for keyboard-only users.

Flags: needinfo?(simona.rosu)

I have retested this issue using the latest Nightly 74.0a1 (Build ID: 20200113204142) and the issue is still reproducible using the steps described in comment 0. I have tested this on Windows 10 x64, Ubuntu 16.04 x64 and MacOS 10.14.
Unfortunately, I haven't observed any changes or improvements regarding this behavior using the latest Nightly 74.0a1 build that contains the fix for bug 1607223.

Flags: needinfo?(simona.rosu)
Assignee: nobody → andy.bruere+oss
Status: NEW → ASSIGNED

I’ve just submitted a fix with a new test for focus trap. I simply moved the overflow: auto rule, waiting for Bug 1604140 (P2) to be fixed.

This new test fails with current login-item component as expected and should keep passing if https://phabricator.services.mozilla.com/D60257 is reverted once Bug 1604140 is fixed.

I’ll be happy to make any change needed so you can land this in the mean time.

Note: I had a browser test rather than a chrome test to be able to detect focus trap.

Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/ad51c58bb7b7
Rework login-item overflow rule to solve focus trap r=MattN
https://hg.mozilla.org/integration/autoland/rev/b7c6f1201537
Add regression test for login-item focus trap r=MattN
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 74

I have verified this issue using the latest Nightly 74.0a1 (Build ID: 20200129093157) on Windows 10 x64, Mac 10.14, Ubuntu 16.04 x64.

  • Keyboard navigation is no longer trapped in the Login Item.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Regressions: 1613580
Depends on: 1615215
You need to log in before you can comment on or make changes to this bug.