1501897 - Permission settings: Only the permission selector for the selected website should be tabbable

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[James Teh [:Jamie]]

STR:
1. Grant permission for at least two websites to send notifications.
2. Open Firefox Options.
3. Select Privacy, then under Permissions -> Notifications, press Settings.
4. Tab to the list of sites, then press down arrow to select the second site in the list.
5. Press tab.
Expected: Focus should land on the selector (menulist.website-status) for the second site.
Actual: Focus lands on the selector for the first site.
6. Press tab again.
Expected: Focus should land on the Remove Website button.
Actual: Focus lands on the selector for the second website.

Since there's a list with selectable items, it doesn't make sense for tab to focus controls associated with other (not selected) websites. For screen readers, this is extremely confusing and could result in a user changing permissions for the wrong site. Aside from that, it also results in a lot of unnecessary tab stops.

Fixing this might be tricky because XUL. In HTML terms, only the permission selector for the currently selected site should have tabindex="0". The others should all be focusable, but not in the tab order (tabindex="-1"). Unfortunately, this is not an HTML control, and IIRC, XUL controls don't support being set to focusable but not tabbable (equivalent to HTML tabindex="-1").

Comment 1

[:Gijs (out for now; he/him)]

(In reply to James Teh [:Jamie] from comment #0)
> Fixing this might be tricky because XUL. In HTML terms, only the permission
> selector for the currently selected site should have tabindex="0". The
> others should all be focusable, but not in the tab order (tabindex="-1").
> Unfortunately, this is not an HTML control, and IIRC, XUL controls don't
> support being set to focusable but not tabbable (equivalent to HTML
> tabindex="-1").

At least from a quick test, using tabindex=-1 seems to wfm.

This doesn't seem like a regression or a very severe issue (you can after all filter the list and/or press [tab] multiple times), so marking as P3.

Comment 2

[James Teh [:Jamie]]

(In reply to :Gijs (he/him) from comment #1)
> This doesn't seem like a regression

It isn't.

> or a very severe issue (you can after
> all filter the list and/or press [tab] multiple times)

For a screen reader user, it can be a pretty severe issue. While screen readers do report "context" (e.g. groupings), list items aren't considered as a candidate for context in most screen readers. So, when a user hits tab, they will only hear, for example, "combo box  allow  collapsed". Since there was no other indication of the item they were adjusting and there was a selectable list before it, the user will (intuitively) assume that they're adjusting for the selected item in the list.

Comment 3

[Johann Hofmann [:johannh]]

Maybe Prathiksha has time to look into this. :)

Comment 4

[:Gijs (out for now; he/him)]

(In reply to Johann Hofmann [:johannh] from comment #3)
> Maybe Prathiksha has time to look into this. :)

Yeah, based on comment #2 I think this is higher priority. Prathiksha, do you have time to look into this? If not, please pass me the needinfo back and I'll try to find someone else.

Comment 5

[:prathiksha]

(In reply to :Gijs (he/him) from comment #4)

> Yeah, based on comment #2 I think this is higher priority. Prathiksha, do
> you have time to look into this? If not, please pass me the needinfo back
> and I'll try to find someone else.

Yeah, I'll take a look at this.

Comment 6

[:prathiksha]

Attachment: Bug 1501897 - Only selected items in the permission settings richlistbox should be tabable for accessibility reasons. r?johannh

Only selected items in the permission settings richlistbox should be tabable for accessibility reasons.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3141b7b79153
Only selected items in the permission settings richlistbox should be tabable for accessibility reasons. r=johannh,Jamie

Comment 8

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df35c50cab3d
Backed out changeset 3141b7b79153 for browser chrome failures on browser_permissions_dialog.

Comment 9

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=3141b7b791532ad0dccdfb058ed693448cd299a2&searchStr=browser%2Cchrome&selectedJob=208901999

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=208901999&repo=autoland&lineNumber=13113

Backout link: https://hg.mozilla.org/integration/autoland/rev/df35c50cab3de855e3b4d085a695bb5826279269

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/654dc5ab446c
Only selected items in the permission settings richlistbox should be tabable for accessibility reasons. r=johannh,Jamie

Comment 11

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/654dc5ab446c

Comment 12

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 65.0a1 (BuildId:20181024221315).

This issue is verified fixed using Firefox 65.0b12 (BuildId:20190117232427) on Windows 10 64bit and Ubuntu 16.04 64bit.