[Affected versions]: Firefox 63.0a1 (BuildId:20180801223951). Firefox 62.0b13 (BuildId:20180730180407). Firefox 61.0.1 (BuildId:20180704003137). Firefox 60.1.0esr (BuildId:20180621121604). [Affected platforms]: Windows 10 64bit. [Preconditions] Enable NVDA screen reader. [Steps to reproduce]: 1. Launch Firefox. 2. Access the following webpage: https://en.wikipedia.org/wiki/Main_Page. 3. Hover over the "Toggle reader view" button. [Expected result]: The screen reader reads the Reader View button on hover. [Actual result]: Screen reader doesn't read the Reader View button on hover. [Regression range]: This seems to be a regression: Last good revision: b8da99694cb6a61ea346e6369333c3b88a283bdc First bad revision: 5ed5cde4cb2c746ece77517c778208a59b2f1de6 Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b8da99694cb6a61ea346e6369333c3b88a283bdc&tochange=5ed5cde4cb2c746ece77517c778208a59b2f1de6

Hi Dao, It seems that mozregression pointed out Bug 1441788 for causing this regression. Can you please take a look into this? Thanks!

(In reply to Emil Ghitta, QA [:emilghitta] from comment #1) > It seems that mozregression pointed out Bug 1441788 for causing this > regression. Jamie, why would switching from a tooltiptext attribute to a tooltip change anything here? Even if it would, there's also a label attribute on the node, which is what I'd expect to be plugging in that information here and providing something to be read...

(In reply to :Gijs (Not available 3-19 Aug; he/him) from comment #2) > Jamie, why would switching from a tooltiptext attribute to a tooltip change > anything here? The a11y engine uses tooltiptext as the name exposed for a11y, or if there's already a name (e.g. label attribute), tooltiptext gets exposed as the a11y description. In contrast, the tooltip attribute points to something that only appears on hover; a11y can't just get the text from it. This is problematic for three reasons: 1. It means the information is only accessible to users if they hover the mouse (and most screen reader users don't). 2. Even if they do hover the mouse, the information only appears in a separate tooltip node. That is, the information isn't exposed on the accessible for the Reader button. The association is purely visual: the tooltip visually appears near the button. 3. NVDA does have support for reading tooltips when they appear, but it only works for native Win32 tooltips for performance reasons. The XUL tooltip attribute is an a11y problem in general. Another example of this is bug 1410757. Can you explain why we prefer this tooltip technique rather than tooltiptext? Is it so the information doesn't have to be calculated ahead of time or is there some other reason? > Even if it would, there's also a label attribute on the node, > which is what I'd expect to be plugging in that information here and > providing something to be read... I don't see a label attribute. From browser.xul: 914 <image id="reader-mode-button" 915 class="urlbar-icon urlbar-page-action" 916 tooltip="dynamic-shortcut-tooltip" 917 role="button" 918 hidden="true" 919 onclick="ReaderParent.buttonClick(event);"/>

(for reply to comment 3 and since Dao fixed bug 1441788)

I could work around this using aria-label, but there seems to be a more fundamental question about the dynamic-shortcut-tooltip implementation from bug 940116.

I think we should keep dynamic-shortcut-tooltip but in UpdateDynamicShortcutTooltipText we can set aria-label on the triggerNode to the same value from the gDynamicTooltipCache. In my testing NVDA will read an updated value of the aria-label attribute.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > I think we should keep dynamic-shortcut-tooltip but in > UpdateDynamicShortcutTooltipText we can set aria-label on the triggerNode to > the same value from the gDynamicTooltipCache. In my testing NVDA will read > an updated value of the aria-label attribute. UpdateDynamicShortcutTooltipText is called from onpopupshowing. Does the popup even show in the first place if you don't trigger it with the mouse?

No, it doesn't. I overlooked that. I'm not even sure how to move keyboard focus to these buttons. I can only get keyboard focus to move to the site-identity button by using Ctrl+L to get focus on the location bar then Shift+Tab to move focus to the site identity buttons.

I'd expect you need to use NVDA to navigate the accessible elements.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #8) > I'm not even sure how to move keyboard focus to these buttons. Currently, you can't. However, I'm working on fixing that in bug 1436086. (In reply to Dão Gottwald [::dao] from comment #9) > I'd expect you need to use NVDA to navigate the accessible elements. You can get to the button with NVDA review commands: 1. Press alt+d to focus the address bar. 2. Repeatedly use the next object command to move through the page actions buttons. This command is NVDA+numpad6 with NVDA set to desktop layout, NVDA+shift+rightArrow when set to laptop layout. "NVDA" is insert or alternatively caps lock if you have the latter enabled in settings. Note that it's probably simpler for most devs to confirm this using the Accessibility Inspector in the Browser Toolbox. If you find the DOM node for the button in the DOM Inspector, you should be able to right click it and select Show Accessibility Properties.

I can confirm using NVDA and the Accessibility pane that there is no label for this button, and I was able to access it using Jamie's steps from comment #10. With Fluent we should be able to remove the UpdateDynamicShortcutTooltipText function since we can build out those tooltips inside of the .ftl files and can most likely remove usage of the dynamic-shortcut-tooltip altogether. Can you use the aria-label workaround for now and then the Fluent conversion project can tackle removing the dynamic-shortcut-tooltip implementation? Looking at UpdateDynamicShortcutTooltipText and the related strings, there are none that are context-specific and would need to be changed depending on external factors such as the target of the item or how many

Did some drive-by cleanup while I was touching this code.

Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/758955ee971a Set aria-label on Reader View button and set menuitem-specific attributes directly there instead of on the command element. r=jaws

This issue is verified fixed using Firefox 64.0a1 (BuildId:20180905223809) on Windows 10 64bit. The NVDA screen reader successfully reads the Reader View button on hover.

Approval Request Comment [Feature/Bug causing the regression]: bug 1441788 [User impact if declined]: see comment 0 [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: [List of other uplifts needed for the feature/fix]: / [Is the change risky?]: no [Why is the change risky/not risky?]: just adding the aria-label attribute to the button [String changes made/needed]: no

Comment on attachment 9006826 [details] [diff] [review] patch for uplift Approved for 63 beta 4

This issue is verified fixed using Firefox 63.0b4 (BuildId:20180906162647) on Windows 10 64bit.

Jamie, is there value in uplifting this to ESR60?

While this is a trivial, very low risk patch for a clear regression, I don't think many users are going to be hurt by this; I haven't seen any complaints at all in the wild. I don't think it's worth uplifting to ESR.