1512411 - NVDA reads the previously hovered "Hamburger" menu option instead of the currently in hovered one

Status: VERIFIED FIXED

(Firefox :: Disability Access, defect)

Description

[Emil Ghitta, QA [:emilghitta]]

Attachment: Reproducible Hover Speed.gif

[Affected versions]:
Firefox 65.0a1 (BuildId:20181205213623).

[Unaffected versions]:
Firefox 64.0 (BuildId:20181203202653).
Firefox 60.3.0esr (BuildId:20181017185317).

[Affected platforms]:
Windows 10 64bit.

[Preconditions]
Enable NVDA screen reader.

[Steps to reproduce]:
1. Launch Firefox.
2. Click on the "Hamburger" menu button.
3. Click on the "Help" button.
4. Hover over the available options a little faster. 

[Expected result]:
NVDA reads the option that is currently being hovered.

[Actual result]:
NVDA sometimes reads the previously hovered option.

Example: Hovering over the "About Nightly" and then over the "Report Deceptive Site" makes NVDA read "About Nightly" even though the currently in hover option is "Report Deceptive Site".

[Regression range]:
This seems to be a recent regression:

Last good revision: 5f543ba66e2c74cfb9ece932447a2fded0da9abb
First bad revision: 111154a7621cc50da7bed70c93171b96c8ef92f0

Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5f543ba66e2c74cfb9ece932447a2fded0da9abb&tochange=111154a7621cc50da7bed70c93171b96c8ef92f0

[Notes]
I have attached a screencast for more info about the "hovering speed" that I used in order to reproduce this issue.
Please note that I have managed to reproduce this issue on the "Hamburger menu", "Page actions" and "Library menu" options.

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Hi Marco, 

It seems that mozregression pointed out Bug 1507365 for causing this regression.

Can you please have a look ? 

Thank you!

Comment 2

[Marco Zehe (:MarcoZ)]

Aside from the fact that a blind screen reader user probably wouldn't use the browser in such a fashion, all that bug does is add proper labeling for toolbar buttons. So some controls might be labelled more that weren't labelled before in that panel. So I suspect that there is some latency involved when NVDA speaks the mouse hover that simply wasn't showing before due to missing labels. Jamie, do you have any idea on how NVDA works internally here?

Comment 3

[James Teh [:Jamie]]

Now that the label accessibles are being exposed as children of the button, NVDA mouse routing hits the label (not the button), since that's the deepest node. That should be fine, except something is failing when we try to get the correct text at those screen coordinates from the IAccessibleText interface of the label. Actually, the IAccessibleText interface on those labels is quite broken in other ways too; e.g. can't review past the first character with NVDA. This only seems to happen on labels created with the label attribute, not labels which are "real" children of the button. (The latter is what bug 1507365 was intended to fix.)

Marco, can you look into this? Failing an understanding of why, we should prevent label children from being exposed in the label attribute case; e.g. maybe we only do the bug 1507365 behaviour if the label attribute is *not* present.

Comment 4

[Marco Zehe (:MarcoZ)]

Emil, could you install this try build and see if you can still reproduce the issue? https://treeherder.mozilla.org/#/jobs?repo=try&revision=e12a68d87e6988a02d657689f8c66559da1d9792

Since you as a sighted person can move the mouse more directly and see the hovered button and hear the one being spoken, you will be able to better reproduce this bug than I, who can only move the mouse in seemingly random places. Thanks!

Comment 5

[Marco Zehe (:MarcoZ)]

Attachment: Bug 1512411 - Don't expose broken label children accessibles on a XUL toolbarbutton which specifies the label attribute, r=Jamie

In bug 1507365, we introduced the ability of nested label children of toolbar buttons to provide the accessible name for a xul:toolbarbutton element. However, the XBL of xul:toolbarbutton causes the creation of a label accessible even if no xul:label child is present, and only the label attribute is being used. The nsIAccessibleText interface on such labels is, however, completely broken.

To fix this, we only accept xul:label children if they come from real XUL markup, not from the label attribute and the XBL creating the label. This means that some of the test changes from bug 1507365 have to be reverted to accomodate the new old behavior. But the new test for an actual label child still passes.

Comment 6

[James Teh [:Jamie]]

This is going to need an uplift to 65 once verified.

Comment 7

[Emil Ghitta, QA [:emilghitta]]

This issue is not reproducible using the try build (provided in comment 4).

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4efb0957f56b
Don't expose broken label children accessibles on a XUL toolbarbutton which specifies the label attribute, r=Jamie

Comment 9

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/4efb0957f56b

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please nominate this patch for Beta approval when you get a chance.

Comment 11

[Marco Zehe (:MarcoZ)]

Comment on attachment 9031207 [details]
Bug 1512411 - Don't expose broken label children accessibles on a XUL toolbarbutton which specifies the label attribute, r=Jamie

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: bug 1507365

User impact if declined: When hovering the mouse over some buttons when NVDA is active, NVDA will no longer see the button's label and be able to speak it.

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: See comment #0 or the Gif attached to the bug.

List of other uplifts needed: None.

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It is more strict on a rule imposed by bug 1507365, and the cases are covered by automated tests (some of which are just reverts of tests introduced by said bug).

String changes made/needed: None.

Comment 12

[Emil Ghitta, QA [:emilghitta]]

This is verified fixed using Firefox 66.0a1 (BuildId:20181217000148) on Windows 10 64bit with NVDA.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9031207 [details]
Bug 1512411 - Don't expose broken label children accessibles on a XUL toolbarbutton which specifies the label attribute, r=Jamie

[Triage Comment]
Improves NVDA's ability to see and read some button labels when hovering over them. Approved for 65.0b5.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d2c26e2f1164

Comment 15

[Emil Ghitta, QA [:emilghitta]]

This is verified fixed using Firefox 65.0b5 (BuildId:20181217180946) on Windows 10 64bit with NVDA.