Closed Bug 1583426 Opened 3 months ago Closed 2 months ago

Up arrow on first item in logins list throws a11y focus to the list itself

Categories

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

70 Branch
defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox70 --- unaffected
firefox71 --- unaffected

People

(Reporter: Jamie, Assigned: jaws)

References

Details

(Keywords: access, Whiteboard: [passwords:management] [skyline][access-p2])

Attachments

(1 obsolete file)

STR (with the NVDA screen reader):

  1. Open about:logins.
  2. Tab to the logins list.
  3. If there are breached logins, press up arrow until you reach the first item.
  4. Press up arrow once more.
    • Expected: NVDA says "Logins matching search query list"
    • Actual: NVDA should say nothing.

Note that tabbing away and back does not restore a11y focus, so this could confuse users.

If you interrogate with the console, aria-activedescendant is set, but it is set to "new-login-list-item". This item is present in the DOM, but is display: none.

Still happy to take a patch here for 70.

Assignee: nobody → jaws
Status: NEW → ASSIGNED
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5af44a2f8201
Clear the aria-activedescendant attribute if navigating through the list reaches an end of the list to trigger NVDA to speak the list's aria-label. Restore the aria-activedescendant attribute upon tabbing back to the listso the... r=sfoster
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

I have followed the steps provided in comment 0 and I get the same results on the latest Nightly 71.0a1 (Build ID 20191003214801) and the Nightly build from Oct 2 where the fix is not present:

  • After focusing the first login from the list if I press the "Up" key the NVDA says "Logins matching search query list".

Based on comment 0 this is the expected behavior. @James can you please confirm this?

The only difference that I have observed in the latest Nightly 71.0a1 build with the fix is that after following the steps, press "Tab" key to focus the next element ("Create New Login" button) and press "Shift"+"Tab" key in order to focus the previously element "Login List", the first login is focused and it is announced by the NVDA. On the older builds, the first login is not announced.

Flags: needinfo?(jteh)
Component: Password Manager → about:logins
Product: Toolkit → Firefox
Target Milestone: mozilla71 → Firefox 71
Version: unspecified → 70 Branch

(In reply to Cosmin Muntean, Experiments QA (in PTO until October 18) from comment #5)

  • After focusing the first login from the list if I press the "Up" key the NVDA says "Logins matching search query list".

Based on comment 0 this is the expected behavior. @James can you please confirm this?

Ug. I'm sorry. It looks like I flipped the "expected" and "actual" results in comment 0. That should have been:

  • Expected: NVDA should say nothing.
    -Actual: NVDA says "Logins matching search query list"

The only difference that I have observed in the latest Nightly 71.0a1 build with the fix is that after following the steps, press "Tab" key to focus the next element ("Create New Login" button) and press "Shift"+"Tab" key in order to focus the previously element "Login List", the first login is focused and it is announced by the NVDA. On the older builds, the first login is not announced.

This is certainly a lot better than it was; at least a user will not get into a broken state from which they can't recover by pressing tab/shift+tab.

:jaws, it looks like aria-activedescendant now gets removed altogether when you press up arrow at the top of the list. However, if you tab then shift+tab, it behaves as expected. I really hope this implementation wasn't a result of my confusing expected/actual reversal. :( Should we reopen this or file a new bug?

Flags: needinfo?(jteh) → needinfo?(jaws)

I in fact went through troubles to try to match your expected/actual results in comment 0. I believe it was already saying nothing prior to the patch after some other refactorings. I figured out that if I removed aria-activedescendant then NVDA would announce the aria-label for the list without also announcing the active descendant.

I don't think it would be easy to back out the patch that landed here so a new bug would be what we would need.

Flags: needinfo?(jaws) → needinfo?(jteh)

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)

I in fact went through troubles to try to match your expected/actual results in comment 0.

Ug. I'm really sorry. I only noticed this after the NI and I went back and read my comment again.

I believe it was already saying nothing prior to the patch after some other refactorings.

Not sure what refactorings you're referring to, but at least in the nightly builds I tested, it was reporting "Logins matching search query list" due to the "new-login-list-item" aria-activedescendant setting.

I'll file a new bug and see if I can fix it myself, seeing as I just led you around the garden path. Sorry again.

Flags: needinfo?(jteh)

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8)

I believe it was already saying nothing prior to the patch after some other refactorings.

It seems the patch for bug 1583425 fixed this. Before that patch, pressing up arrow was reporting "Logins matching search query list". After that patch, this worked as expected (nothing reported).

I don't think it would be easy to back out the patch that landed here so a new bug would be what we would need.

I locally backed this patch out from both central and beta and it backs out cleanly. I verified that everything works as expected after the backout. Can we just request a backout here (from both central and beta) or do we need a new bug?

Sorry again for the trouble.

Flags: needinfo?(jaws)

Thanks for looking in to the backout. I clearly wasn't that confident in it working but if it backs out cleanly then we should just proceed with backing out since it is effectively fixed by the other bug. We can use this bug for the backout.

@Liz, can you help coordinate a backout of this patch from beta and central?

Flags: needinfo?(jaws) → needinfo?(lhenry)
Depends on: 1587103
Status: RESOLVED → REOPENED
Flags: needinfo?(jaws)
Resolution: FIXED → ---
Target Milestone: Firefox 71 → ---
Flags: needinfo?(lhenry)
Status: REOPENED → RESOLVED
Closed: 2 months ago2 months ago
Resolution: --- → WORKSFORME
Attachment #9098366 - Attachment is obsolete: true

Thanks all, and final apology for the trouble.

Depends on: 1583425

(In reply to James Teh [:Jamie] from comment #13)

Thanks all, and final apology for the trouble.

No worries, mistakes happen. Thanks for your extra attention to this new feature to make sure that we ship an accessible feature from the beginning.

I have verified this issue on the Firefox Beta 70.0b13 (Build ID: 20191007220302) and on the latest Nightly 71.0a1 (Build ID: 20191008214557) build on Windows 10 x64.

  • I confirm that backout of this bug is performed.
  • The initial behavior is still reproducible.
You need to log in before you can comment on or make changes to this bug.