1363026 - accessibility tree not accessible to object inspection in latest firefox

Status: RESOLVED DUPLICATE of bug 1422674

(Core :: Disability Access APIs, defect)

Description

[steve faulkner]

In Firefox 51/52 on windows object inspection tools such as inspect.exe and aViewer cannot access the accessibility tree correctly. Child nodes of the currently inspected node are not available.

related aViewer bug: https://github.com/ThePacielloGroup/aviewer/issues/26

Comment 1

[steve faulkner]

note: i tried the tools with Screen reader running before opening Firefox, then opened inspect tool. Made no difference.

Comment 2

[Marco Zehe (:MarcoZ)]

It is something that these tools do differently than screen readers. NVDA and JAWS (at least in single-process mode of Firefox) continued to work normally after Firefox 50, to this day in Nightly builds. My guess is that the tools access stuff out of process whereas screen readers do a lot of the magic in-process.

Including Aaron, too, so he may be able to shed some light on this.

One important thing to note is that no official version on Windows supports multi-process Firefox yet with accessibility on. Normally, when accessibility is triggered, it should turn off multi-process mode, but it is worth checking if that checkbox under Tools/Options/General is checked or unchecked that talks about tabs in multiple processes.

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

I use inspect all the time when testing e10s mode.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

Too late to fix in 53, if this is a Firefox bug. Aaron says that we should be testing to get more details from inspect.exe, here. I'm also not clear if this is related to e10s-multi or not. 
Emil, are you able to reproduce this issue in 55? Can you give some STR if you can?

Comment 5

[Emil Pasca [:emilpasca]]

I have manged to reproduce the issue on latest Firefox Release(53.0.2) and the latest Nightly (55.0a1-20170510030301) with the following steps:

[Prerequisites]:
Download and install aViewer tool from https://www.paciellogroup.com/resources/aviewer/

[Steps to reproduce]:
1) Open the Accessibility Viewer.
2) Select the "Watch focus"(F4) and the "Watch Cursor"(F5) tools.
3) Open the browser and navigate to any webpage.
4) Hover with the cursor on any page element.
5) Observe the node tree displayed in the tree view.

[Notes]
- Attached a screen recording of the issue: https://goo.gl/zQgeJ9

I have performed a regression here are is the result:
Last good revision: 9a4d30b1d480e94f19dd0fa4d156c8f3fda53f0a
First bad revision: 7cc58d0708e7ee5f0c06bf0ef760a591153a6fef
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9a4d30b1d480e94f19dd0fa4d156c8f3fda53f0a&tochange=7cc58d0708e7ee5f0c06bf0ef760a591153a6fef

Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1304449

Aaron can you please take a look at this?
Do let me know if there's anything I can help with. Thank you!

Comment 6

[Marco Zehe (:MarcoZ)]

Thank you, Emil!

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

What about inspect.exe? I am more interested in that than aviewer.

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

inspect.exe looks fine.

I am resolving this as WONTFIX. It looks to me like aviewer is trying to enumerate children by calling IAccessible::get_accChild with a sequential positive integer. That is assuming that everything is a simple element and that the server has assigned positive sequential ids to everything, neither of which are required in general [1][2].

This did work prior to bug 1304449, but my overhaul of child IDs removed that functionality; we no longer expose simple element child IDs to the external APIs.

I suggest querying the IAccessible for IEnumVARIANT [3] instead (this is what inspect.exe does). If you just wanted to retrieve the IDs, you could use AccessibleChildren [4]. Both of these allow you to enumerate children without making assumptions about the underlying child ID implementation.

If there is a real compelling case made that the simple element case should be supported, I think we'd need to run that by davidb and the rest of the team, but from where I sit, this scheme was never guaranteed to be supported by every single IAccessible object. Because of e10s, any implementation of positive sequential IDs would probably requires us needing to call [3] or [4] to resolve the real IAccessible anyway... we might as well cut out the middle-man.

[1] https://msdn.microsoft.com/en-us/library/ms971310.aspx
[2] https://msdn.microsoft.com/en-us/library/windows/desktop/dd373643(v=vs.85).aspx
[3] https://msdn.microsoft.com/en-us/library/windows/desktop/ms221053(v=vs.85).aspx
[4] https://msdn.microsoft.com/en-us/library/windows/desktop/dd317975(v=vs.85).aspx

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

This does not block a11y+e10s rollout.

Comment 10

[Marco Zehe (:MarcoZ)]

Thanks Aaron, I agree it does not block. Also thanks for your good explanation of the issue and how to fix it on aviewer's end.

Comment 11

[Trevor Saunders (:tbsaunde)]

> I am resolving this as WONTFIX. It looks to me like aviewer is trying to
> enumerate children by calling IAccessible::get_accChild with a sequential
> positive integer. That is assuming that everything is a simple element and
> that the server has assigned positive sequential ids to everything, neither
> of which are required in general [1][2].

do you mean iterating from 1 to childCount to get direct children?  It seems to me breaking that is risky and if we want to do that we should update comments and remove the code that tries to do that.

Or do you mean something else?  If this is about returning objects in some other case I probably don't care and I'm not sure how it managed to work before.

I kind of assume the latter because I would expect the code we have for the former works, and if it doesn't should be straight forward to fix up with the one exception of  accessibles that are the parent of a top level document in a content process.

> This did work prior to bug 1304449, but my overhaul of child IDs removed
> that functionality; we no longer expose simple element child IDs to the
> external APIs.

I can never remember the weird terms MSAA uses for its kind of crazy setup.  So I'm not sure what you mean here off hand.

> I suggest querying the IAccessible for IEnumVARIANT [3] instead (this is
> what inspect.exe does). If you just wanted to retrieve the IDs, you could
> use AccessibleChildren [4]. Both of these allow you to enumerate children
> without making assumptions about the underlying child ID implementation.

it seems reasonable and likely that somebody out there does accessible->get_accChild(i : [1, childCount])

> If there is a real compelling case made that the simple element case should
> be supported, I think we'd need to run that by davidb and the rest of the
> team, but from where I sit, this scheme was never guaranteed to be supported
> by every single IAccessible object. Because of e10s, any implementation of
> positive sequential IDs would probably requires us needing to call [3] or
> [4] to resolve the real IAccessible anyway... we might as well cut out the
> middle-man.

not sure what to think here without understanding better what we are talking about.

Comment 12

[alexander :surkov (:asurkov)]

(In reply to Aaron Klotz [:aklotz] (a11y work receiving priority right now) from comment #8)
> inspect.exe looks fine.
> 
> I am resolving this as WONTFIX. It looks to me like aviewer is trying to
> enumerate children by calling IAccessible::get_accChild with a sequential
> positive integer. That is assuming that everything is a simple element and
> that the server has assigned positive sequential ids to everything, neither
> of which are required in general [1][2].
> 
> This did work prior to bug 1304449, but my overhaul of child IDs removed
> that functionality; we no longer expose simple element child IDs to the
> external APIs.

It was always used to be that get_accChild worked in two modes
1) positive numbers as index in parent
2) negative number as unique id of a child

If the first options is not longer supported, then it looks like a regression on our side.

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

Steve did you work around this on your side?

Comment 14

[alexander :surkov (:asurkov)]

(In reply to Aaron Klotz [:aklotz] (a11y work receiving priority right now) from comment #8)
> inspect.exe looks fine.

> This did work prior to bug 1304449, but my overhaul of child IDs removed
> that functionality; we no longer expose simple element child IDs to the
> external APIs.

I'm curious though if it was intentional change, IAccessible::get_childAt(childIndex) and IAccessible2::indexInParent are supposed to work in a bunch. Reopening the bug.

Comment 15

[(No longer employed by Mozilla) Aaron Klotz]

That's fine, we can restore that behaviour easily enough, however I stand by my assertion that aViewer should not be using positive child IDs as a generic mechanism for enumerating children.

Comment 16

[steve faulkner]

(In reply to David Bolter [:davidb] from comment #13)
> Steve did you work around this on your side?

Yes, fixed in latest aViewer release https://github.com/ThePacielloGroup/aviewer/releases/tag/2.0.3

Comment 17

[alexander :surkov (:asurkov)]

(In reply to Aaron Klotz [:aklotz] (a11y work receiving priority right now, please send interceptor reviews to dmajor or handyman) from comment #15)
> That's fine, we can restore that behaviour easily enough, however I stand by
> my assertion that aViewer should not be using positive child IDs as a
> generic mechanism for enumerating children.

because it is slower than IEnumVariant allowing to grab children at once or there are other reasons?

Comment 18

[(No longer employed by Mozilla) Aaron Klotz]

The existence of the "positive id as sequential 1-based index" scheme is not guaranteed by MSAA. According to MSDN (as I linked in comment 8):

"To differentiate between the elements sharing an IAccessible object, the server assigns a unique, positive child identifier to each simple element. This assignment is done on a per-instance-of-interface basis, so the IDs must be unique within that context. Many implementations assign these IDs sequentially, beginning with 1. This scheme does not allow simple elements to have children of their own."

Note that it says that "many" implementations assign sequential positive IDs, not "all" implementations.

While I could conceivably see some ATs depending on our previous behaviour for compat reasons, as a general tool to be used for enumerating accessible objects, I don't think the existence of that scheme should be assumed.

Comment 19

[David Bolter [:davidb] (NeedInfo me for attention)]

Let's close this bug out as Steve has fixed this on his end. Let's file a new one if we discover other breakage.

Comment 20

[James Teh [:Jamie]]

I actually fixed this in bug 1422674.