Closed Bug 878409 Opened 11 years ago Closed 11 years ago

crash in nsAccessiblePivot::SearchForward @ nsIContent::HasAttr

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Version:
24 Branch
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla24
Tracking Flags:
Tracking Status
firefox23 --- unaffected
firefox24 --- fixed

People

(Reporter: MarcoZ, Assigned: eeejay)

References

Details

(Keywords: crash, regression, Whiteboard: [native-crash])

Crash Data

Attachments

(1 file)

Fix crash in RuleCache::ApplyFilter() when accessible has no content node.
1.17 KB, patch
tbsaunde
: review+
surkov
: feedback+
Details | Diff | Splinter Review 
This bug was filed from the Socorro interface and is 
report bp-037de6ef-3e37-42c5-96e0-ebb2c2130601 .
=============================================================
It has been bit by two users since 24.0a1/201305130 so likely a recent regression.
My best guess is bug 869280.

Stack trace:
Frame 	Module 	Signature 	Source
0 	libxul.so 	nsIContent::HasAttr const 	obj-firefox/dist/include/nsINode.h:1359
1 	libxul.so 	mozilla::a11y::nsAccUtils::HasDefinedARIAToken 	nsAccUtils.cpp:187
2 	libxul.so 	RuleCache::ApplyFilter 	nsAccessiblePivot.cpp:585
3 	libxul.so 	nsAccessiblePivot::SearchForward 	nsAccessiblePivot.cpp:495
4 	libxul.so 	nsAccessiblePivot::MoveNext 	nsAccessiblePivot.cpp:216
5 	libxul.so 	nsAccessiblePivot::MovePrevious 	nsAccessiblePivot.cpp:245
6 	libxul.so 	NS_InvokeByIndex 	xptcinvoke_arm.cpp:164
7 	libxul.so 	XPCWrappedNative::CallMethod 	XPCWrappedNative.cpp:2938
8 	org.mozilla.fennec-2.apk 	org.mozilla.fennec-2.apk@0xb70007 	
9 	libxul.so 	js::NewFunction 	jsobjinlines.h:1627
10 	libxul.so 	XPCNativeScriptableShared::PopulateJSClass 	XPCWrappedNativeJSOps.cpp:1409
11 	libxul.so 	js::NewFunctionByIdWithReserved 	jsfriendapi.cpp:466
12 	dalvik-heap (deleted) 	dalvik-heap @0x3dcac4e 	
13 	dalvik-mark-stack (deleted) 	dalvik-mark-stack @0x4912c1e 	
14 	libxul.so 	DefinePropertyById 	jsobjinlines.h:1131 

More reports at:
https://crash-stats.mozilla.com/report/list?signature=nsIContent%3A%3AHasAttr%28int%2C+nsIAtom*%29+const
status-firefox23: --- → unaffected
status-firefox24: --- → affected
Keywords: regression
Hardware: All → ARM
Summary: crash in nsIContent::HasAttr → crash in nsAccessiblePivot::SearchForward @ nsIContent::HasAttr
Version: unspecified → 24 Branch
Whiteboard: [native-crash]
The nsIContent passed to nsAccUtils::HasDefinedARIAToken() is probably null. THere is an assertion there, but I don't see it in the reports. I guess those don't get included?
Attachment #757451 - Flags: feedback?(surkov.alexander)
Comment on attachment 757451 [details] [diff] [review]
Fix crash in RuleCache::ApplyFilter() when accessible has no content node.

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

::: accessible/src/base/nsAccessiblePivot.cpp
@@ +582,5 @@
>  
>      if (nsIAccessibleTraversalRule::PREFILTER_ARIA_HIDDEN & mPreFilter) {
>        nsIContent* content = aAccessible->GetContent();
> +      if (content &&
> +          nsAccUtils::HasDefinedARIAToken(content, nsGkAtoms::aria_hidden) &&

scenario?

document? if so don't you want to process <body aria-hidden="true"> case?
(In reply to alexander :surkov from comment #4)
> Comment on attachment 757451 [details] [diff] [review]
> Fix crash in RuleCache::ApplyFilter() when accessible has no content node.
> 
> Review of attachment 757451 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/base/nsAccessiblePivot.cpp
> @@ +582,5 @@
> >  
> >      if (nsIAccessibleTraversalRule::PREFILTER_ARIA_HIDDEN & mPreFilter) {
> >        nsIContent* content = aAccessible->GetContent();
> > +      if (content &&
> > +          nsAccUtils::HasDefinedARIAToken(content, nsGkAtoms::aria_hidden) &&
> 
> scenario?
> 

I'm not sure. Obviously mContent in nsAccessNode could be null, since it is checked for elsewhere.

> document? if so don't you want to process <body aria-hidden="true"> case?

If we can't retrieve the content, there is no node, and no point in checking for an attribute. I'm not sure what you mean here.
(In reply to Eitan Isaacson [:eeejay] from comment #5)
> (In reply to alexander :surkov from comment #4)
> > Comment on attachment 757451 [details] [diff] [review]
> > Fix crash in RuleCache::ApplyFilter() when accessible has no content node.
> > 
> > Review of attachment 757451 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: accessible/src/base/nsAccessiblePivot.cpp
> > @@ +582,5 @@
> > >  
> > >      if (nsIAccessibleTraversalRule::PREFILTER_ARIA_HIDDEN & mPreFilter) {
> > >        nsIContent* content = aAccessible->GetContent();
> > > +      if (content &&
> > > +          nsAccUtils::HasDefinedARIAToken(content, nsGkAtoms::aria_hidden) &&
> > 
> > scenario?
> > 
> 
> I'm not sure. Obviously mContent in nsAccessNode could be null, since it is
> checked for elsewhere.

right, for example, it can be null for documents. Otherwise it shouldn't be.

> > document? if so don't you want to process <body aria-hidden="true"> case?
> 
> If we can't retrieve the content, there is no node, and no point in checking
> for an attribute. I'm not sure what you mean here.

ignore it
Attachment #757451 - Flags: feedback?(surkov.alexander) → feedback+
Attachment #757451 - Flags: review?(surkov.alexander)
Comment on attachment 757451 [details] [diff] [review]
Fix crash in RuleCache::ApplyFilter() when accessible has no content node.

f+,r+ is too much from one person :) let's try Trev, maybe he will spot something yet
Attachment #757451 - Flags: review?(surkov.alexander) → review?(trev.saunders)
(In reply to alexander :surkov from comment #6)
> (In reply to Eitan Isaacson [:eeejay] from comment #5)
> > (In reply to alexander :surkov from comment #4)
> > > Comment on attachment 757451 [details] [diff] [review]
> > > Fix crash in RuleCache::ApplyFilter() when accessible has no content node.
> > > 
> > > Review of attachment 757451 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > ::: accessible/src/base/nsAccessiblePivot.cpp
> > > @@ +582,5 @@
> > > >  
> > > >      if (nsIAccessibleTraversalRule::PREFILTER_ARIA_HIDDEN & mPreFilter) {
> > > >        nsIContent* content = aAccessible->GetContent();
> > > > +      if (content &&
> > > > +          nsAccUtils::HasDefinedARIAToken(content, nsGkAtoms::aria_hidden) &&
> > > 
> > > scenario?
> > > 
> > 
> > I'm not sure. Obviously mContent in nsAccessNode could be null, since it is
> > checked for elsewhere.
> 
> right, for example, it can be null for documents. Otherwise it shouldn't be.

that's the only case I can think of that matters here if I'm correct in thinking you don't care about defunct things here.
Attachment #757451 - Flags: review?(trev.saunders) → review+
Blocks: 869280
https://hg.mozilla.org/mozilla-central/rev/20ce72ffb6fb
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: