1652439 - Crash in [@ mozilla::a11y::ServiceProvider::QueryService]

Status: RESOLVED FIXED

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

Description

[Marco Zehe (:MarcoZ)]

This bug is for crash report bp-f9fcbc49-ef28-4db6-833e-c888b0200712.

Top 2 frames of crashing thread:

0 xul.dll mozilla::a11y::ServiceProvider::QueryService accessible/windows/msaa/ServiceProvider.cpp:67
1 ole32.dll Invoke com\ole32\com\txf\callframe\amd64\stubless.asm:289

Reported by Léonie Watson on Twitter.

Second report mentioned by Léonie, which reports the same signature: bp-dc0e4362-2102-48e5-b2ca-dc27d0200712.

Comment 1

[James Teh [:Jamie]]

I don't know how to reproduce this with JAWS, but STR with the NVDA Python console:

1. Open this URL:
   data:text/html,test
2. Ensure the document has focus.
3. Press NVDA+control+z to open the NVDA Python console.
4. Paste the following command as one line:
   import comtypes, IAccessibleHandler; nav.IAccessibleObject.QueryInterface(IAccessibleHandler.IAccessible2_2).relationTargetsOfType("nodeChildOf", 1)[0][0].QueryInterface(comtypes.IServiceProvider).QueryService(comtypes.GUID("{a5d8e1f3-3571-4d8f-9521-07ed28fb072e}"), IAccessibleHandler.IAccessible)
   • Expected: No tab crash. Console output: _ctypes.COMError: (-2147467262, 'No such interface supported', (None, None, None, 0, None))
   • Actual: Tab crash. Console output: _ctypes.COMError: (-2147023170, 'The remote procedure call failed.', (None, None, None, 0, None))

JAWS queries nodeChildOf on the top level document. We return the ApplicationAccessible... which is kinda weird anyway, but fixing that is tricky and JAWS doesn't seem to care. However, JAWS then sometimes seems to QueryService for SID_IAccessibleContentDocument. It's this "sometimes" bit I can't figure out: I've never seen it do that myself on the parent of the top level document (and thus can't reproduce the crash), but the fact that users are seeing this crash suggests that it does sometimes.

Comment 2

[James Teh [:Jamie]]

Attachment: Bug 1652439: Null check the root accessible when handling QueryService to SID_IAccessibleContentDocument in content processes.

Accessible::RootAccessible() returns null on the ApplicationAccessible.
We weren't null checking this previously, so QueryService for SID_IAccessibleContentDocument on the ApplicationAccessible in a content process would crash.
This is rare, but JAWS does sometimes trigger it.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5eaaa416b436
Null check the root accessible when handling QueryService to SID_IAccessibleContentDocument in content processes. r=MarcoZ

Comment 4

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5eaaa416b436

Comment 5

[James Teh [:Jamie]]

Verified in latest nightly with the NVDA python Console as per the STR in comment 1. I was never able to reproduce this with JAWS (which is the scenario that impacts users). Even so, I'd like to get this into beta to get some feedback from more JAWS users.

Comment 6

[James Teh [:Jamie]]

Comment on attachment 9163397 [details]
Bug 1652439: Null check the root accessible when handling QueryService to SID_IAccessibleContentDocument in content processes.

Beta/Release Uplift Approval Request

• User impact if declined: Crashes for JAWS screen reader users.
• 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: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Simple null check.
• String changes made/needed:

Comment 7

[James Teh [:Jamie]]

[Tracking Requested - why for this release]: Multiple crashes a day for JAWS screen reader users. If we did have to push another 78.0.x release, we think this should be included.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

I assume we want this on ESR78 also.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9163397 [details]
Bug 1652439: Null check the root accessible when handling QueryService to SID_IAccessibleContentDocument in content processes.

Approved for 79.0b9 and 78.1esr.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/10b143a974ea

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/dabc162949f7

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Fx79 is in RC, so wontfix for 78.