Closed Bug 1486887 Opened Last year Closed Last year

Crash in mozilla::dom::Element::GetAttr

Categories

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

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed

People

(Reporter: marcia, Assigned: eeejay)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file, 2 obsolete files)

This bug was filed from the Socorro interface and is
report bp-ada89000-2b32-4cfb-85d4-f66060180828.
=============================================================

Seen while looking at nightly crash stats: https://bit.ly/2BXCX5i. Crashes started using 20180823100106.

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3b5452b3778153f6f223bb2177235835b2314eb6&tochange=49b70f7e6817131d79876ab88667bc6c2833e4a0

It looks as if some of this code was touched in Bug 1485862, although that seems to have landed after the regression started.

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::Element::GetAttr dom/base/Element.cpp:2926
1 xul.dll mozilla::a11y::HTMLTextFieldAccessible::NativeAttributes accessible/html/HTMLFormControlAccessible.cpp:314
2 xul.dll mozilla::a11y::Accessible::Attributes accessible/generic/Accessible.cpp:979
3 xul.dll mozilla::a11y::ia2Accessible::get_attributes accessible/windows/ia2/ia2Accessible.cpp:489
4 xul.dll void mozilla::a11y::HandlerProvider::BuildDynamicIA2Data accessible/ipc/win/HandlerProvider.cpp:315
5 xul.dll void mozilla::a11y::HandlerProvider::BuildInitialIA2Data accessible/ipc/win/HandlerProvider.cpp:389
6 xul.dll nsresult mozilla::detail::RunnableMethodImpl<mozilla::a11y::HandlerProvider*, void  xpcom/threads/nsThreadUtils.h:1219
7 xul.dll void `anonymous namespace'::SyncRunnable::APCRun ipc/mscom/MainThreadInvoker.cpp:64
8 xul.dll static void mozilla::mscom::MainThreadInvoker::MainThreadAPC ipc/mscom/MainThreadInvoker.cpp:187
9 ntdll.dll RtlDispatchAPC 

=============================================================
It appears FindFirstNonChromeOnlyAccessContent may return a null, which is not suggested by its name. Eitan, do you have thoughts on this one?
Flags: needinfo?(eitan)
Priority: -- → P2
I this happens when an entire doc is chrome only. It just traverses up the tree and never finds a non-chrome content. We can fallback to mContent in that case.
Flags: needinfo?(eitan)
OK, so that last patch was pretty bad. I never really understood all of this well.
Hopefully, this is right. Looks like we already do stuff like this with the
obsoletely named XULWidgetElm().
Attachment #9005278 - Flags: review?(surkov.alexander)
Assignee: nobody → eitan
Comment on attachment 9005278 [details] [diff] [review]
Use binding parent for input[type] retrieval. r?surkov

Review of attachment 9005278 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nits addressed

::: accessible/html/HTMLFormControlAccessible.cpp
@@ +307,5 @@
>    // Expose type for text input elements as it gives some useful context,
>    // especially for mobile.
>    nsAutoString type;
> +  // In the case of this element being part of a binding like
> +  // input[type=number], the binding's parent's type should have precedence.

is it because <input type="number"> contains <input> as anonymous content? Could you change the comment to reflect the widget structure to answer why the binding's parent's type should take precedence.

@@ +308,5 @@
>    // especially for mobile.
>    nsAutoString type;
> +  // In the case of this element being part of a binding like
> +  // input[type=number], the binding's parent's type should have precedence.
> +  nsIContent* widgetElm = XULWidgetElm();

nit: could you please change XULWidgetElm() to return dom::Element as you go?
Attachment #9005278 - Flags: review?(surkov.alexander) → review+
Attachment #9005278 - Attachment is obsolete: true
I didn't change the return type because I can't be 100% certain it will always be an Element.
Keywords: checkin-needed
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2531be10419
Use binding parent for input[type] retrieval. r=surkov
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e2531be10419
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
See Also: → 1525546

Comment on attachment 9042015 [details]
Bug 1525546 - Make sure a XULMenuItemAccessibleWrap is not going away when creating its keyboard shortcut info, r=Jamie

Sorry, totally messed up the bug number since I was looking at this bug when creating a patch for another one. Sorry for the noise!

Attachment #9042015 - Attachment is obsolete: true
Attachment #9042015 - Attachment description: Bug 1486887 - Make sure a XULMenuItemAccessibleWrap is not going away when creating its keyboard shortcut info, r=Jamie → Bug 1525546 - Make sure a XULMenuItemAccessibleWrap is not going away when creating its keyboard shortcut info, r=Jamie
Attachment #9042015 - Attachment is obsolete: false

Comment on attachment 9042015 [details]
Bug 1525546 - Make sure a XULMenuItemAccessibleWrap is not going away when creating its keyboard shortcut info, r=Jamie

Revision D18922 was moved to bug 1525546. Setting attachment 9042015 [details] to obsolete.

Attachment #9042015 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.