NVDA takes on the dev tools document as the content of certain tabs

RESOLVED FIXED in Firefox 58

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: derek.riemer, Assigned: Jamie)

Tracking

({regression})

unspecified
mozilla59
x86
Windows 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox57 unaffected, firefox58 fixed, firefox59 verified)

Details

(Whiteboard: aes+)

Attachments

(1 attachment)

1. Run the NVDA screen reader.
2. open a new tab, type random text in the address bar, and perform a search.
3. Now, press f12. Now, press shift+tab to focus the document.
4. control+t time!
5. Type random text into the address bar again, and search for it.
6. Try to read the resulting document with NVDA.

Expected: NVDA reads the search engines document.
Actual: NVDA reads the dev tools document as if it were actually the page.
Confirmed. Once you focus the Dev Tools console output area, browse mode for all documents in the same content process reflects the content of the console output, not the content document.

This occurs because IAccessible::accChild for the Dev Tools output document answers for child ids in all content documents in that process. NVDA browse mode uses accChild to test whether an accessible is part of a document, so in this case, Firefox is essentially telling NVDA that everything belongs to the console output document.

I don't quite understand why this is happening yet. This Dev Tools document seems to be special somehow; it doesn't happen anywhere else.
Status: UNCONFIRMED → NEW
Component: Developer Tools → General
Ever confirmed: true
Product: Firefox → Core
Version: Trunk → unspecified
Component: General → Disability Access APIs
Could the fact that the webconsole is an HTML document and not XUL be an issue? Does this happen on any other devtools like the Inspector or Style Editor?
The fact that it's an HTML document is certainly relevant (because NVDA browse mode gets used for it). NVDA browse mode being used for the document in question is a condition needed to trigger the behaviour.

What process does the console run in? Does it run in the parent process, the content process for the document it is inspecting or some other process?
Could the fix for bug 1414451 be at fault here? An undesired side effect?
(In reply to James Teh [:Jamie] from comment #3)
> What process does the console run in? Does it run in the parent process, the
> content process for the document it is inspecting or some other process?

The devtools UI runs in the parent process, loaded in an iframe in browser.xul (and sub iframes for each tool). So something like:

browser.xul:
-- toolbox.xul:
---- webconsole.html
---- inspector.xhtml
---- debugger.html
---- styleeditor.xul
Jamie confirmed by backing out bug 1414451 from a local build that this is indeed the culprit. Setting flags accordingly. Note that 58 is affected because bug 1414451 was uplifted there to fix dev tools extensions accessibility and responsive design view.
The point about it being related to the same content process was a red herring. The confusion was because whether we expose this bug depends on the order of the documents in NVDA's browse mode documents set. In reality, accChild on all documents in the parent process responds to ids for all accessibles in remote processes.

Here's a reliable way to reproduce this with the NVDA python console:

1. Open mozilla.org
2. Open a new tab and load about:support
3. Focused on the about:support document, press NVDA+control+z.
4. Save off the about:support document by entering: doc = focus
5. Switch back to Firefox.
6. Switch to the mozilla.org tab.
7. Focused on the mozilla.org document, press NVDA+control+z.
8. Enter: nav in doc
Expected: False
Actual: True
CC: eeejay
The change made in bug 1414451 was that when getting remote accessibles, we compare the root of the accessible being called against the root of the remote document. Previously, we compared the *documents* of the two accessibles.

In terms of this bug, this means that previously, we compared the local document (since the document of a document is the document itself) against the root (since the document of an outer doc is the root). These are not equal and thus we skip the remote document. Now, we compare the root of the local document and the root of the outer doc, which are the same. Thus, we do query the remote document and end up getting an accessible.

While the previous behaviour was better for this bug, it was still somewhat broken. accChild on most chrome accessibles that weren't inside another document would return remote documents that weren't descendants. For example, if you called accChild on the Navigation toolbar and passed the id of a remote document, it would happily return an accessible, which is still wrong.

There are only two cases in which accChild is used with a negative id:
1. To retrieve a specific accessible from an event or a cached id (always called on the root); or
2. To test whether something is a descendant of a document (always called on the document and should fail for anything which isn't in that document).
For case 2, accChild is called on the remote document and it already won't answer for an id which isn't a descendant.
The simplest way to support case 1 without breaking case 2 is to only support querying remote ids on the root accessible.
Assignee: nobody → jteh
Comment on attachment 8934059 [details]
Bug 1422201: Only handle remote ids passed to IAccessible::accChild on the root accessible.

https://reviewboard.mozilla.org/r/205002/#review210448
Attachment #8934059 - Flags: review?(mzehe) → review+
Pushed by mzehe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/81958c54d557
Only handle remote ids passed to IAccessible::accChild on the root accessible. r=MarcoZ
Testing performed:
1. Confirmed (expected) failure when calling accChild on a local document, passing a remote id. Previously, this succeeded when it shouldn't.
2. Confirmed (expected) failure when calling accChild on the Navigation toolbar , passing a remote id. Previously, this succeeded when it shouldn't.
3. Confirmed success when calling accChild on the root accessible, passing a remote id. No change.
4. Confirmed success when calling accChild on the root accessible, passing a local id. No change.
5. Confirmed success when calling accChild on the root accessible, passing an id within a document beneath the responsive design view. No change, but this was what bug 1414451 was fixing.

I'm seeing inconsistent results (both before and after the patch) regarding accChild with positive child ids. I need to investigate that further.
(In reply to James Teh [:Jamie] from comment #13)
> I'm seeing inconsistent results (both before and after the patch) regarding
> accChild with positive child ids. I need to investigate that further.

Filed bug 1422674.
https://hg.mozilla.org/mozilla-central/rev/81958c54d557
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Verified fixed in Nightly 59.0a1 20171204234137.
Please request uplift to beta when you get a chance.
Comment on attachment 8934059 [details]
Bug 1422201: Only handle remote ids passed to IAccessible::accChild on the root accessible.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1414451.
[User impact if declined]: Screen reader users will be unable to access web content properly when a chrome document (web console, about:support, etc.) is open.
[Is this code covered by automated tests?]: No, because we don't have a mechanism for platform accessibility testing.
[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]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Trivial patch which only affects accessibility. Easy to explain, justify and verify the change in behaviour.
[String changes made/needed]: None.
Attachment #8934059 - Flags: approval-mozilla-beta?
Comment on attachment 8934059 [details]
Bug 1422201: Only handle remote ids passed to IAccessible::accChild on the root accessible.

Fix an accessibility regression issue. Beta58+.
Attachment #8934059 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3176ff271bd7ee01b48faa09
d93590e902245975&tochange=81958c54d5571136c5cb5eb0df71bb5c9748a6fa

1. Try to open grease monkey, or lastpass from the bar of addons.
Expected: a browse mode document with NVDA.
Actual: an unknown.
Output from mozregression:

23:56.09 INFO: First bad revision: 81958c54d5571136c5cb5eb0df71bb5c9748a6fa
23:56.09 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=3176ff271bd7ee01b48faa09
d93590e902245975&tochange=81958c54d5571136c5cb5eb0df71bb5c9748a6fa
Depends on: 1424657
You need to log in before you can comment on or make changes to this bug.