Closed Bug 1329616 Opened 3 years ago Closed 3 years ago

Massive increase of focus loss and Unknown Accessibles problems with E10S turned off starting in 2016-12-16 Nightly

Categories

(Core :: Widget: Win32, defect, P1, major)

53 Branch
x86
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox52 + fixed
firefox53 --- fixed

People

(Reporter: MarcoZ, Assigned: aklotz)

References

()

Details

(Keywords: regression, Whiteboard: aes+, tpi:+)

Attachments

(1 file)

STR:
1. Start Firefox in non-E10S mode, and have NVDA running.
2. Go to Gmail's inbox.
3. Arrow up and down.

Result: Every few lines, NVDA will not speak the newly highlighted conversation item, but speak the document or window name and then something about "unknown".

4. Press up or down arrow.

Result: NVDA may, or may not, revert to speaking the newly highlighted conversation, or continue to speak Unknown for at least one more arrow key press.

Notes:
1. This does not happen with E10S on.
2. It is not confined to Gmail, but happens on many web sites, including Bugmail, a directory listing of archive.mozilla.org when tabbing through, etc., etc.

Last good build is 2016-12-15, first bad nightly build is 2016-12-16. Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=7652a58efa46f1c57c94bba26efc5d53b6184e83&tochange=63b447888a6469b9f6ae8f76ac5f0d7c6ea239da

Primary suspect is Bug 1323521: Prevent Windows touchscreen support from instantiating a11y. There is one more a11y-related patch in that period, but that's a patch by Alex for the a11y DOM work, which does not appear to be of impact here.

Aaron, could you take a look to see if there's a chance that this could be happening? The symptoms look like a wm_getobject failing even though a11y has already be instanciated.

I am on Windows 10 on a Microsoft Surface Book, and it does have a touch screen. No other Windows machine without a touch screen currently available, so I cannot test whether this behaves differently on a non-touchscreen device.
Flags: needinfo?(aklotz)
[Tracking Requested - why for this release]: Regression from bug 1323521, which landed on 52 to minimize impact of E10S a11y on Windows touch screen users. However, for screen reader users, this has adverse effects in non-e10s mode, resulting in a largely unusable browser with lots of focus loss issues. I confirmed that this bug affects current versions of 52.0a2 Aurora.
Whiteboard: aes+
Tracking this a11y regression for 52.
The problem is that UIA calls SendMessageTimeout(WM_GETOBJECT) on behalf of tiptsf. Fine, that one we want to block. But when SendMessageTimeout executes, it also flushes other pending SendMessage calls.

This means that there could potentially be more than one WM_GETOBJECT invocation. One of those should be suppressed as the tiptsf request, but the others need to be let through.
Flags: needinfo?(aklotz)
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Component: Disability Access APIs → Widget: Win32
Yes, this appears to fix the problem. Thanks!
Flags: needinfo?(mzehe)
A few hours later I can confirm that earlier statement with more certainty. Have been running the try build in E10S mode off for my whole morning doing day to day work, and it is indeed working as it should.
Depends on: 1325676
Attached patch PatchSplinter Review
Comment 3 outlines the cause of the problem.

While debugging this, I noticed that WM_GETOBJECT messages are always initiated via SendMessageTimeout from within our own main thread.

The easiest way to take care of this is to bypass the window manager altogether: We can examine the SendMessageTimeout and respond to WM_GETOBJECT messages immediately.

Unfortunately this solution also suffers from the dependency on bug 1328996: We can only do it on 32-bit builds at the moment.
Attachment #8825512 - Flags: review?(jmathies)
(In reply to Aaron Klotz [:aklotz] from comment #8)
> Unfortunately this solution also suffers from the dependency on bug 1328996:
> We can only do it on 32-bit builds at the moment.

To be clear: This patch fixes the immediate problem in this bug on both 32-bit and 64-bit builds, however it also changes the a11y-blocking code such that it only works on 32-bit builds.
Attachment #8825512 - Flags: review?(jmathies) → review+
Priority: -- → P1
Whiteboard: aes+ → aes+, tpi:+
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c346814acd16e7ef980de6c36f61d1ae4d38607
Bug 1329616: Use intercepted SendMessageTimeoutW to check for WM_GETOBJECT originating from tiptsf; r=jimm
https://hg.mozilla.org/mozilla-central/rev/6c346814acd1
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment on attachment 8825512 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1323521
[User impact if declined]: a11y problems in both e10s and non-e10s modes
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: This depends on the patch from bug 1325676 being landed first.
[Is the change risky?]: No
[Why is the change risky/not risky?]: Simple, well understood patch.
[String changes made/needed]: None
Attachment #8825512 - Flags: approval-mozilla-aurora?
Comment on attachment 8825512 [details] [diff] [review]
Patch

a11y regression fix for aurora52
Attachment #8825512 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
needs rebasing for aurora
Flags: needinfo?(aklotz)
Depends on: 1334257
You need to log in before you can comment on or make changes to this bug.