Closed Bug 1776498 Opened 2 years ago Closed 2 years ago

shift f10 no longer works with Jaws

Categories

(Core :: Disability Access APIs, defect, P1)

Product:
Core
Component:
Disability Access APIs
Version:
Firefox 103
Platform:
Desktop
Windows
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
104 Branch
Tracking Flags:
Tracking Status
firefox-esr91 --- unaffected
firefox-esr102 --- fixed
firefox102 --- wontfix
firefox103 + verified
firefox104 + verified

People

(Reporter: covici, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1776498 - Use popup class with all popups. r=Jamie
48 bytes, text/x-phabricator-request
dmeehan
: approval-mozilla-release+
RyanVM
: approval-mozilla-esr102+
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:103.0) Gecko/20100101 Firefox/103.0

Steps to reproduce:

use the shift-f10 or applications key

Actual results:

Jaws did not read the context menu -- its on there but Jaws isn't seeing it This is Jaws 2022 June update

Expected results:

Jaws should read the context menu choices

Now this bug seems to have made its way to the released version firefox 102. I have gotten a number of reports, so please pri this up so it can be fixed.

Thanks.

Moving to a more appropriate component.

Component: Untriaged → Disability Access APIs
Keywords: access
Product: Firefox → Core

I haven't confirmed this yet, but I'm prioritising it assuming I can reproduce it.

Assignee: nobody → jteh
Severity: -- → S2
OS: Unspecified → Windows
Priority: -- → P1
Hardware: Unspecified → Desktop
Keywords: access

This seems to be similar to Bug 1581955.
I've managed to reproduce it on the latest versions Nightly 104.0a1 and Firefox 102.0.1 on Windows10 x64.

I've narrowed down the regression range to:
Last good revision: 39ded09a9c01993586a1652659ce66e27032d54f
First bad revision: f8b149335cd2b8ee8e8a8ca2fa7b59ebca890067
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=39ded09a9c01993586a1652659ce66e27032d54f&tochange=f8b149335cd2b8ee8e8a8ca2fa7b59ebca890067

Status: UNCONFIRMED → NEW
Has STR: --- → yes
status-firefox102: --- → affected
status-firefox103: --- → affected
status-firefox104: --- → affected
status-firefox-esr102: --- → affected
status-firefox-esr91: --- → unaffected
Ever confirmed: true

Thanks so much for tracking down the regression.

This was caused by the changes in bug 1768278. Before that, the window class for popup menus was MozillaDropShadowWindowClass. Now, it's MozillaWindowClass. I can't be sure since I don't have access to the JAWS source code, but my guess is that they depend on this class to handle Mozilla popup menus for some reason. This kind of hard-coding wouldn't surprise me with JAWS; we've seen various instances of such hard-coding over the years.

We could ask the JAWS vendor to adjust this, but it's likely that this would take months to get to users. Also, because of the cost of updates, many users will be on older versions of JAWS for years to come. So, unfortunately, we're going to need to hack around this on our side somehow.

We do have a flag for JAWS which we could expose to XPCOM. Emilio, would it be feasible (albeit super ugly) to set an attribute on menupopups based on this JAWS flag and have the CSS react accordingly, reverting to the old behaviour if the attribute is set? How hard would the CSS changes be?

Flags: needinfo?(emilio)
Keywords: regression
Regressed by: 1768278

I've been trying to come up with a way to hack around this within the a11y engine rather than polluting other code. So far, I've tried:

  1. Making a11y use the root HWND for menupopups instead of the popup HWND. JAWS still ignores the event.
  2. Firing a focus win event on the menupopup when it opens. JAWS does seem to pay attention to this - it at least stops the virtual cursor from intercepting the arrow keys - but it still doesn't report the presence of the context menu.
  3. Hooking GetClassNameW to return "MozillaDropShadowWindowClass" for popup windows. Unfortunately, our DLL interceptor can't hook that function; we get:
    Assertion failure: false (MOZ_ASSERT_UNREACHABLE: Unrecognized opcode sequence), at C:/Users/jamie/src/gecko/obj/dist/include/mozilla/interceptor/PatcherDetour.h:1208

The only other way I can think of to hack around this in a11y is to create a fake MozillaDropShadowWindowClass HWND, similar to our HWND emulation for MozillaContentWindowClass. That is going to be tricky for two reasons:

  1. That window class is managed by widget code. At the very least, we'll have to override the window proc set by the class, though we can probably do that with SetWindowLong.
  2. The window class might not have been created yet (nsWindow::GetWindowPopupClass might not have been called). That method is private, so we'll either need to make it public or figure out some other way to use it.

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

The only other way I can think of to hack around this in a11y is to create a fake MozillaDropShadowWindowClass HWND,
...

  1. That window class is managed by widget code. At the very least, we'll have to override the window proc set by the class, though we can probably do that with SetWindowLong.

Blah. No, we can't, because SetWindowLong can only be called after the window is created, but WM_CREATE is sent before CreateWindow returns. That means some messages will have already been sent to widget's window proc before we can override the window proc with our own. This is not going to work.

If we were going to revert to the old behaviour when JAWS is running, we'd also have to revert bug 1768285.

That leaves me with a question, though. Per nsMenuPopupFrame::CreateWidgetForView, I think we use MozillaDropShadowWindowClass if !(mode == eTransparencyTransparent || tag == nsGkAtoms::menulist). When do we ever create a menu popup that isn't transparent and isn't a menulist? Is MozillaDropShadowWindowClass even used any more?

If it isn't, we could make it the same as normal Mozilla widgets and just use that class for all menupopups for the express purpose of satisfying JAWS. That does mean somehow telling the widget code whether something is a menupopup, which feels ugly and will probably require shoving something new in widgetData. Yuck.

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

Thanks so much for tracking down the regression.

This was caused by the changes in bug 1768278. Before that, the window class for popup menus was MozillaDropShadowWindowClass. Now, it's MozillaWindowClass.

Before that we used MozillaWindowClass depending on various -moz-window-shadow values, which means that this couldn't have worked before in things like HCM (since we didn't use drop shadows there...).

We do have a flag for JAWS which we could expose to XPCOM. Emilio, would it be feasible (albeit super ugly) to set an attribute on menupopups based on this JAWS flag and have the CSS react accordingly, reverting to the old behaviour if the attribute is set? How hard would the CSS changes be?

I don't think making CSS changes is worth it here. We should always be able to use the shadow class for all menupopups.

Flags: needinfo?(emilio)

Does something like this seem reasonable?

Donal, I understand we're really close to the 103 release, so we probably won't get this fixed there. However, this is severe enough that I think we should try to get it into a dot release if there is one. What flags should I set and/or what else should I do for that to be considered?

Flags: needinfo?(dmeehan)

[Tracking Requested - why for this release]: Makes Firefox context menus inaccessible to users of the JAWS screen reader.

tracking-firefox104: --- → ?

(In reply to Emilio Cobos Álvarez (:emilio) from comment #9)

Before that we used MozillaWindowClass depending on various -moz-window-shadow values, which means that this couldn't have worked before in things like HCM (since we didn't use drop shadows there...).

Do you mean that if HCM was enabled, we would always have used MozillaWindowClass, which in turn means context menus would have been broken for JAWS users who also had HCM enabled? I guess the set of users who have both enabled is likely to be pretty low - JAWS users tend to either be totally blind or have limited enough vision that HCM is not useful for their needs - which would explain why we've never seen this get reported before.

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

Donal, I understand we're really close to the 103 release, so we probably won't get this fixed there. However, this is severe enough that I think we should try to get it into a dot release if there is one. What flags should I set and/or what else should I do for that to be considered?

We can track it for 103 and 104.
I'll change 103 tracking to fix-optional. 103 is now in RC. When you have landed a patch, if I get a release uplift request I can consider it as a ride-along for an RC respin or dot release (depending on risk, etc.)

status-firefox103: wontfix → fix-optional
tracking-firefox103: --- → +
tracking-firefox104: ? → +
Flags: needinfo?(dmeehan)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/18a7f69d5ce4
Use popup class with all popups. r=Jamie,handyman

https://hg.mozilla.org/mozilla-central/rev/18a7f69d5ce4

Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox104: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 104 Branch

:jamie if you are confident with this and add a release uplift request, I can consider it as a ride along in an RC respin or dot release

Flags: needinfo?(jteh)

I tried to reproduce the issue before the fix on Win10x64 using Firefox build 102.0.1(20220705093820) and indeed the Shift +F10 did nothing on Jaws.
I also tried on latest Nightly 104.0a1(20220724190402) and also Shift +F10 did nothing but right click on main page did mention Context menu which did not happen before the fix - is this the intended behavior? Thank you so much.

When you say shift+f10 did nothing, I assume it didn't bring up the context menu visually either? I'm not sure why that would be; perhaps the page wasn't focused. That said, if JAWS spoke the context menu correctly when you right clicked, that verifies the fix.

One other concern we have here is whether this affects the visual presentation of context menus and tooltips in Windows 7 and Windows 8. Is that something you'd be able to test for us? If all is well, there should be no difference in the visual appearance of context menus and tooltips in Windows 7 or 8 both before and after the fix.

Thanks.

Flags: needinfo?(jteh) → needinfo?(monica.chiorean)

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

When you say shift+f10 did nothing, I assume it didn't bring up the context menu visually either? I'm not sure why that would be; perhaps the page wasn't focused. That said, if JAWS spoke the context menu correctly when you right clicked, that verifies the fix.

No it did not bring the context menu visually and the page was in focus.
Only if I press right click, Jaws mentions Context Menu and to navigate between options it warns me to press up/down arrows. - This was also the behavior on an older version of FF 100.0a1(20220315213924) before the issue was reported. Verified same behavior is on Win10x64 using latest Beta104.0b1.

One other concern we have here is whether this affects the visual presentation of context menus and tooltips in Windows 7 and Windows 8. Is that something you'd be able to test for us? If all is well, there should be no difference in the visual appearance of context menus and tooltips in Windows 7 or 8 both before and after the fix.

Yes we will help you with the verification on Win7 and Windows 8 (VM), but we have a few questions:

  1. Do we need to check the fix with Jaws on Win7/Win8? (As a note Jaws offers only 40 mins of free demo).
  2. Context menu and tooltips need verification using Jaws on Win7/8 ?
  3. For tooltips what tooltip do you think of? (tooltips for Context menu, links, menu options)?
  4. Do we also need to verify the popup menus? If yes, which ones?
  5. For Win8, will be ok to use virtual machine 8.1?
Flags: needinfo?(monica.chiorean) → needinfo?(jteh)

(In reply to Monica Chiorean from comment #21)

  1. Do we need to check the fix with Jaws on Win7/Win8? (As a note Jaws offers only 40 mins of free demo).

No. I'm confident the fix will work with JAWS. The concern is now only whether there are any visual issues.

  1. Context menu and tooltips need verification using Jaws on Win7/8 ?

No.

  1. For tooltips what tooltip do you think of? (tooltips for Context menu, links, menu options)?

Any tooltip displayed when you hover in Firefox UI. For example, let's use the tooltips that appear when you hover Firefox toolbar buttons.

  1. Do we also need to verify the popup menus? If yes, which ones?

Verifying the page context menu and a submenu inside it (e.g. the Languages submenu) should be sufficient.

  1. For Win8, will be ok to use virtual machine 8.1?

Yes.

Thank you.

Flags: needinfo?(jteh)
Regressions: 1781747

In order to verify what you asked us, we:

  1. Created a test run with existing test cases for context menus with the following report see link (https://testrail.stage.mozaws.net/index.php?/reports/view/5644).
  2. Did a quick check of the tooltips displayed on the default tab from the toolbar.
  3. Added all the buttons from Customize to the toolbar and checked the tooltips on Light and Dark theme.

Result: Except the issue mentioned at regression (bug 1781747) we did not find any other issue.

Is there anything else we need to check? Thank you.

status-firefox104: fixed → verified

There's nothing else that needs to be checked here. Thank you for the fast turnaround.

Comment on attachment 9285792 [details]
Bug 1776498 - Use popup class with all popups. r=Jamie

Beta/Release Uplift Approval Request

  • User impact if declined: Context menus are inaccessible to users of the JAWS screen reader.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: Bug 1781747 (follow-up fix).
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Fix has been verified by QA.
  • String changes made/needed:
  • Is Android affected?: No
Attachment #9285792 - Flags: approval-mozilla-release?
Assignee: jteh → emilio

Comment on attachment 9285792 [details]
Bug 1776498 - Use popup class with all popups. r=Jamie

Approved for 103.0.2, thanks.

Attachment #9285792 - Flags: approval-mozilla-release? → approval-mozilla-release+

Please nominate this and bug 1781747 for ESR102 approval when you get a chance.

Flags: needinfo?(emilio)

Verified that context menu is heard with Jaws on Win10 using 103.0.2 (20220808125904).

status-firefox103: fixed → verified

Comment on attachment 9285792 [details]
Bug 1776498 - Use popup class with all popups. r=Jamie

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes a regression in 102 for JAWS screen reader users.
  • User impact if declined: JAWS shortcuts don't work.
  • Fix Landed on Version: 103
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): Not the simplest patch, but QA has done verification to be upliftable to release along with the only regression that was found from it.
Flags: needinfo?(emilio)
Attachment #9285792 - Flags: approval-mozilla-esr102?

Comment on attachment 9285792 [details]
Bug 1776498 - Use popup class with all popups. r=Jamie

Approved for 102.2esr.

Attachment #9285792 - Flags: approval-mozilla-esr102? → approval-mozilla-esr102+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: