1518054 - Moving to first or last item in Downloads list does not fire accessible focus event

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[James Teh [:Jamie]]

STR (with the NVDA screen reader):
1. Ensure you have at least three downloads in your downloads history.
2. Press control+j to open the Downloads window.
3. Press down arrow.
Observe: The second download is spoken.
4. Press up arrow.
Expected: The first download should be spoken.
Actual: Only "not selected" is spoken.
5. Press down arrow.
Expected: The second download should be spoken.
Actual: Only "selected" is spoken.
6. Press end.
Expected: The last download should be spoken.
Actual: Only "not selected" is spoken.

This doesn't seem to happen in all richlistbox controls; I also tested the richlistbox of categories in the Options dialog (#categories) and the list of application handlers in Options (#handlersView) and those seem to work as expected. However, I'm filing this in XUL because it seems to be a regression caused by a core XUL change:

38:49.61 INFO: Last good revision: 1bb44497f6c0601c16de02b9595402ccc661b29a
38:49.62 INFO: First bad revision: 0c7a54d4cc426d989d5759f962c8c0af737d5a5b
38:49.62 INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1bb44497f6c0601c16de02b9595402ccc661b29a&tochange=0c7a54d4cc426d989d5759f962c8c0af737d5a5b

This indicates bug 1492326 as the cause of the regression.

Curiously, when this occurs, switching to a different app and back again or opening and then dismissing a menu fires focus on the correct (first or last) item. That would suggest that the DOMMenuItemActive event fired when the list is focused is working just fine:
https://searchfox.org/mozilla-central/source/toolkit/content/widgets/richlistbox.js#105
but something is going awry with the DOMMenuItemActive event fired when the current item changes while the list is already focused:
https://searchfox.org/mozilla-central/source/toolkit/content/widgets/richlistbox.xml#111

Comment 1

[Tim Spurway [:tspurway]]

hi neil - could we get this triaged?

Comment 2

[Neil Deakin]

I think this is a case where the richlistitem's current property is being assigned before the binding has been applied. That causes the first item to never receive DOMMenuItemActive/Inactive events. I tracked this down to being related to _prependBatchFragment within allDownloadsView.js. Disabling most of that function makes the events fire properly as the richlistbox is not removed and re-added into the document. I can't confirm if this is actually the same problem as described above, but it does fix the firing of events in any case.

An alternate fix is to move the event firing from the richlistitem.current implementation into the richlistbox.currentItem setter.

Possibly, also, bug 1512432 might just happen to fix this.

I don't know what the priority should be set to -- this would be an accessibility decision.

Comment 3

[Marco Zehe (:MarcoZ)]

Well, this is a regression users will run into each time they deal with the downloads folder, so I'd say this is at least a P2, if not P1. Jamie, what are your thoughts?

Comment 5

[James Teh [:Jamie]]

From an accessibility perspective, this is a p1, since it effectively makes the first and last items inaccessible to most users.

Comment 6

[James Teh [:Jamie]]

(In reply to Neil Deakin from comment #2)

I think this is a case where the richlistitem's current property is being assigned before the binding has been applied. That causes the first item to never receive DOMMenuItemActive/Inactive events.

That makes sense. However, why would bug 1492326 have changed this behaviour? That is, why was the binding applied earlier before those patches?

Comment 7

[James Teh [:Jamie]]

Attachment: Bug 1518054: Move a11y focus event firing from richlistitem.current to richlistbox.currentItem to fix the All Downloads view.

The All Downloads view removes and re-adds its richlistbox for performance reasons.
However, after bug 1492326, this causes the richlistitem's .current property to be assigned before its binding is applied.
Since the .current property fires a11y focus events, this means this property is overridden and thus the events never get fired for that item.
To fix this, move a11y focus event firing into richlistbox.currentItem.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c46ba0dfc91c
Move a11y focus event firing from richlistitem.current to richlistbox.currentItem to fix the All Downloads view. r=paolo

Comment 9

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/c46ba0dfc91c

Comment 10

[:Paolo Amadini]

Based on the discussion in the review, setting qe-verify+ not only to check this bug specifically, but also for some exploratory testing to see if there are any other issues in the Downloads View in the Library standalone window after bug 1492326.

Comment 11

[laszlo.bialis]

Hi,

I've reproduced the issue using NVDA on Nightly 66.0a1 20190104214806 on Win10 x64, also verified it using NVDA on the latest Nightly 66.0a1 20190122215349 and the issue is not reproducible. Please mention if testing is needed in other OS's also.

Regarding Comment 10, I've made the exploratory testing in the Downloads View in the Library standalone window, on the latest Nightly, comparing the results to the latest Release 62.0.2 (as this was unaffected by the issue mentioned in this bug) on Win10 x64, but I couldn't find any differences nor any other faulty readings.