Handler <--> Proxy Interface Mismatch

RESOLVED FIXED in Firefox 58

Status

()

Core
Disability Access APIs
P1
normal
RESOLVED FIXED
6 months ago
4 months ago

People

(Reporter: aklotz, Assigned: aklotz)

Tracking

Trunk
mozilla58
Unspecified
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fix-optional, firefox58 fixed)

Details

(Whiteboard: aes+)

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

6 months ago
The handler expects accessible proxies to be IAccessible2_3. If we export a different interface from content, the handler has to make an additional round-trip to obtain the correct one.

We should ensure that the interceptor correctly sends the expected interface so that we can avoid this extra work.
(Assignee)

Updated

6 months ago
Depends on: 1393589
(Assignee)

Comment 1

6 months ago
Created attachment 8902482 [details] [diff] [review]
Patch

This patch brings into line the IID that is expected by the client and the IID that is being exported by the interceptor.

This significantly reduces inter-process QI traffic from our browser process because the expected vs actual interfaces now always match.
Attachment #8902482 - Flags: review?(jmathies)

Updated

6 months ago
Attachment #8902482 - Flags: review?(jmathies) → review+
(Assignee)

Comment 2

6 months ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7f2eaf99d46116172266f2486a3682f845fc236
Bug 1393600: Prevent mismatches between the handler's expected interface and the one being provided by content; r=jimm
https://hg.mozilla.org/mozilla-central/rev/e7f2eaf99d46
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57

Updated

6 months ago
Depends on: 1395840
(Assignee)

Comment 4

6 months ago
Reopening due to bug 1395840. I'm going to back this out.
Status: RESOLVED → REOPENED
Keywords: leave-open
Resolution: FIXED → ---
(Assuming this is P1)
Priority: -- → P1
(Assignee)

Comment 7

6 months ago
Marco, could you please try out the try builds in comment 6 once they're ready? Thanks.
Flags: needinfo?(mzehe)

Comment 8

6 months ago
I get an immediate crash of the tab, *any* tab, when running that try build. The crash i submitted is:
https://crash-stats.mozilla.com/report/index/61f0dab0-fea8-48c9-a480-8dca81170913

This is the 64 bit build, of course without symbols.
Flags: needinfo?(mzehe) → needinfo?(aklotz)
(Assignee)

Comment 9

6 months ago
Was this unzipped or via the installer? You'll need to use the installer on this one, sorry for not mentioning that.
Flags: needinfo?(aklotz) → needinfo?(mzehe)

Comment 10

6 months ago
It was the zipped one. Will try again. ;) Thanks!

Comment 11

6 months ago
Still no luck. Symptoms are the same as before: Firefox behaves as if it was a very basic MSAA app. No IA2 related stuff is working or exposed. In the accessible tree, explored via NVDA's object navigation, I see no documents for loaded web pages, and the control information about the currently focused item (NVDA-key+F1) only gives me basic MSAA info, as if Firefox never had heard about any IAccessible2.
Flags: needinfo?(mzehe)

Comment 12

5 months ago
Tried the test builds on Windows 10 build 15063. Confirming Marco's findings in comment 11 with e10s enabled.
(Assignee)

Comment 13

5 months ago
OK, I've got another build to try. Same as last time, use the installer please.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=91ff69e4c27f8d56be9c391f4c0fb92bb4887514
Flags: needinfo?(mzehe)

Comment 14

5 months ago
(In reply to Aaron Klotz [:aklotz] (a11y work receiving priority right now, please send interceptor reviews to dmajor or handyman) from comment #13)
> OK, I've got another build to try. Same as last time, use the installer
> please.

Unfortunately still the same problem. Any remote content has no IAccessible2 and siblings support, just basic MSAA. Local content like about:support, however, works fine with IAccessible2 support.
Flags: needinfo?(mzehe)
(Assignee)

Comment 15

5 months ago
(In reply to Marco Zehe (:MarcoZ) from comment #14)
> Unfortunately still the same problem. Any remote content has no IAccessible2
> and siblings support, just basic MSAA. Local content like about:support,
> however, works fine with IAccessible2 support.

How's this for annoying: Everything works fine on my local test build!

I wonder if this is some kind of strange thing like the previous version's handler is still loaded somewhere and thus the new version's handler isn't updated properly?

Comment 16

5 months ago
Well, I uninstalled the regular Nightly from my machine, from what used to be the Control Panel applet, then installed the test build from the installer. I even tried the 32 bit one, same results. So the old handler must have disappeared from emory and disc when I uninstalled it.
Flags: needinfo?(aklotz)
(Assignee)

Comment 17

5 months ago
David is going to try this out. See Comment 11 and Comment 13.
Flags: needinfo?(aklotz) → needinfo?(dbolter)

Comment 18

5 months ago
1. Once you start the try build with NVDA running, check if virtual buffers are working. Make sure you are on a page from remote, not something like about:support.
2. Press NVDA+F1.
3. In the log viewer that appears, you get a bunch of info about the current object.
4. Scroll down to the last line.
a) If it says IAccessibleValue, you see the bug that I'm seeing.
b) If you see IAccessible2Attributes, you are not seeing my problem.
Using steps in comment 18 I also don't see IAccessible2 info in the nvda log on the 64 bit build from comment 13.
Flags: needinfo?(dbolter)
Setting 57 status to fix-optional since we will be making a special case final call on windows e10s a11y support early in beta cycle.
status-firefox57: fixed → fix-optional
(Assignee)

Comment 21

5 months ago
I was able to reproduce this on my Surface Book. For some reason NVDA is not sending the command to load the Gecko vbuf backend.
(Assignee)

Comment 23

5 months ago
Created attachment 8910972 [details] [diff] [review]
Patch (r2)

I think I found the problem. I changed one spot in the code that I shouldn't have.

Carrying forward r+ since it's the same patch less one diff chunk.
Attachment #8902482 - Attachment is obsolete: true
Attachment #8910972 - Flags: review+
(Assignee)

Comment 24

5 months ago
As for why I could not reproduce it locally, I believe it is because I had previously registered IA2 in the registry. By default this isn't necessary.
(Assignee)

Comment 25

5 months ago
Still problems without that block though. More thought needs to go into this.

Comment 26

5 months ago
This doesn't really help, but just noting a couple of things here because I was curious and investigated them:

1. I was wondering why NVDA (out-proc) can't get the right interface even after the proxy gets injected. The answer is simple: NVDA doesn't support IAccessible2_3 yet; our proxy doesn't know about it.
2. I was wondering why IAccessible worked but IAccessible2 didn't. I figured that if IAccessible2 didn't work, nothing should work. The answer is that we're not actually using IAccessible on the child objects; accChild fails, so we never get them. We end up falling back to calling accName, etc. on the root accessible, but passing the unique id as the child id. Something to remember when dealing with this kind of thing in future: be sure to check the IAccessibleChildID reported in NVDA's dev info. If it shows a non-0 value, you're not dealing with the child IAccessible; accChild failed.
(Assignee)

Comment 27

5 months ago
Created attachment 8914130 [details] [diff] [review]
Part 2: Use IAccessible when exporting to external processes

We need to use the existing IAccessible2_3 interface pointer for exporting to an external process, but that process may not recognize IA2. So we overwrite the IID in the serialized proxy with one that the client *will* recognize: IAccessible.
Attachment #8914130 - Flags: review?(jmathies)
(Assignee)

Comment 29

5 months ago
Alright Marco, let's try the one in comment 28!
Flags: needinfo?(mzehe)

Updated

5 months ago
Attachment #8914130 - Flags: review?(jmathies) → review+

Comment 30

5 months ago
This build works as expected, in that it provides IAccessible2 in the manner NVDA expects. :) I'll reserve judgement on the perf improvement, since this try build seems to be generally very slow for me for some reason.

So I suggest landing these two, and I'll retest the actual perf improvement on a regular nightly build.
Flags: needinfo?(mzehe)
(Assignee)

Comment 31

5 months ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9cf1a5354ac88a80b32ff3276a7a71229d7db06
Bug 1393600: Prevent mismatches between the handler's expected interface and the one being provided by content; r=jimm

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c07fa7775c903bd6c80db11f734b5f1af39f45f
Bug 1393600: Ensure that the handler sends a known interface to external clients; r=jimm

Comment 33

5 months ago
Aaron, can this be closed now, or do you have follow-ups still?
Flags: needinfo?(aklotz)
(Assignee)

Comment 34

5 months ago
Ah, yes, thanks.
Status: REOPENED → RESOLVED
Last Resolved: 6 months ago5 months ago
status-firefox58: --- → fixed
Flags: needinfo?(aklotz)
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: mozilla57 → mozilla58
You need to log in before you can comment on or make changes to this bug.