Closed Bug 878409 Opened 12 years ago Closed 12 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: 12 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: