Closed Bug 1451673 Opened 2 years ago Closed 2 years ago

Crash in mozilla::a11y::HTMLTableAccessible::IsProbablyLayoutTable

Categories

(Core :: Disability Access APIs, defect, P2, critical)

60 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- wontfix
firefox59 --- unaffected
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: philipp, Assigned: surkov)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-9dc48fe0-da75-456c-a908-1808f0180405.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::a11y::HTMLTableAccessible::IsProbablyLayoutTable accessible/html/HTMLTableAccessible.cpp:1066
1 xul.dll mozilla::a11y::HTMLTableAccessible::NativeAttributes accessible/html/HTMLTableAccessible.cpp:448
2 xul.dll mozilla::a11y::Accessible::Attributes accessible/generic/Accessible.cpp:932
3 xul.dll mozilla::a11y::ia2Accessible::get_attributes accessible/windows/ia2/ia2Accessible.cpp:489
4 xul.dll mozilla::a11y::HandlerProvider::BuildDynamicIA2Data accessible/ipc/win/HandlerProvider.cpp:341
5 xul.dll mozilla::a11y::HandlerProvider::BuildInitialIA2Data accessible/ipc/win/HandlerProvider.cpp:415
6 xul.dll mozilla::detail::RunnableMethodImpl<mozilla::a11y::HandlerProvider*, void  xpcom/threads/nsThreadUtils.h:1200
7 xul.dll mozilla::mscom::MainThreadInvoker::MainThreadAPC ipc/mscom/MainThreadInvoker.cpp:187
8 ntdll.dll RtlDispatchAPC 
9 ntdll.dll KiUserApcDispatcher 

=============================================================

content crashes with this signature see to get somewhat more common during the 60.0b cycle.
Hm, looks like the frame for the row is NULL when trying to check for its background color. Xidorn, any idea when this can be the case? You introduced this particular code in part 2 of bug 1326209.

The only thing that changed in beta 60 is that now, it is more compatible with the JAWS screen reader. Don't know if they are triggering this somehow. NI'ing Jamie just in case.
Flags: needinfo?(xidorn+moz)
Flags: needinfo?(jteh)
The change in bug 1326209 isn't relevant here. The previous code also need to read from the computed value for getting the background.

As you said, most likely it is the frame being null. Sounds like the a11y tree loses its frame somehow?

I don't have idea either, as I'm not quite familiar with a11y code...
Flags: needinfo?(xidorn+moz)
Attached patch patchSplinter Review
null check + assertion in hope of hitting this by fuzzing
Assignee: nobody → surkov.alexander
Attachment #8965396 - Flags: review?(mzehe)
Attachment #8965396 - Flags: review?(mzehe) → review+
Keywords: checkin-needed
Priority: -- → P2
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5a6db1121fce
"Crash in mozilla::a11y::HTMLTableAccessible::IsProbablyLayoutTable" r=MarcoZ
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/5a6db1121fce
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Is this worth an uplift request?
Flags: needinfo?(surkov.alexander)
(In reply to Marco Zehe (:MarcoZ) from comment #1)
> The only thing that changed in beta 60 is that now, it is more compatible
> with the JAWS screen reader. Don't know if they are triggering this somehow.

This is just triggered by a query for IA2 attributes while building the handler cache when retrieving an accessible. If JAWS is somehow exposing this more than other clients, it's most likely timing related.
Flags: needinfo?(jteh)
Comment on attachment 8965396 [details] [diff] [review]
patch

Approval Request Comment
[Feature/Bug causing the regression]: unknown
[User impact if declined]: crashes
[Is this code covered by automated tests?]:no
[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]:no
[Is the change risky?]:no
[Why is the change risky/not risky?]: a null check
[String changes made/needed]: no
Flags: needinfo?(surkov.alexander)
Attachment #8965396 - Flags: approval-mozilla-beta?
Comment on attachment 8965396 [details] [diff] [review]
patch

null check to avoid a11y-related crashes, approved for 60.0b12
Attachment #8965396 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1456049
You need to log in before you can comment on or make changes to this bug.