346936 - crashes [@ nsAccessibilityService::CreateRootAccessible]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The #3 unfixed topcrash on the 1.8 branch is crashes at nsAccessibilityService::CreateRootAccessible .  These are occurring on both Windows and Linux.  A few of the comments refer to hitting the back button.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: example talkback incident

Comment 2

[Aaron Leventhal]

Attachment: 1) Fail to return an accessible if document has no container, 2) Don't use pageshow to fire doc loaded events, we don't need it

From the stack trace, I can see that by far the most likely reason for this is because of fastback. A pageshow event is fired very early, before the document owns a docshell. 

The accesssibility event handling code gets the pageshow event and tries to create an accessible for the document, which crashes because of the null docshell.

It turns out that we should not even be listening for pageshow. We have 2 methods for tracking page loads. We use:
1) DOMContentLoaded which results in TryFireEarlyLoadEvent()
2) WebProgressListener which is for the fallback case where there was a framed page or something that doesn't work well if you fire an early load event

So this is a double fix. With this patch, we will ignore the pageshow event. It also adds a null check so that accessibles are not created if for documents with no docshell.

I have tested with this patch on Windows, and we still fire the page load events even when alt+left is pressed during another page load.

Comment 3

[Aaron Leventhal]

Attachment: 1) Fail to return an accessible if document has no container, 2) Don't use pageshow to fire doc loaded events, we don't need it

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 231871 [details] [diff] [review]
1) Fail to return an accessible if document has no container, 2) Don't use pageshow to fire doc loaded events, we don't need it

sr=dbaron, assuming you've tested fastback restoration cases carefully

Comment 5

[Mike Schroepfer]

Comment on attachment 231871 [details] [diff] [review]
1) Fail to return an accessible if document has no container, 2) Don't use pageshow to fire doc loaded events, we don't need it

a=schrep for drivers.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

This caused a new (topcrash?) crash on trunk.
Apparently target is occasionally nsnull
http://lxr.mozilla.org/seamonkey/source/accessible/src/base/nsRootAccessible.cpp#321

Comment 7

[François Gagné]

Should this get also fixed on 1.8.0.x ?

On 1.5.0.7 it's crash #44 with 583 crashes.

Comment 8

[Daniel Veditz [:dveditz]]

1.8.0.8 is almost exclusively for security-fix parity with 1.8.1, the next "stability" release is 1.8.0.9 and we can consider this then. I'm also concerned about the new crash mentioned in comment 6 -- has a bug been filed? Do we need to include that fix as well? I guess that's the bug 352709 dependency bug (In general please mention this sort of thing in comments so we can recontruct that info if the dependency field is ever botched).

Comment 9

[Daniel Veditz [:dveditz]]

We'll leave this one alone on the 1.8.0 branch, we can live with #44 since people who are bothered by it can upgrade.