crash [@ nsAccessible::NativeState()]

VERIFIED FIXED in Firefox 5

Status

()

defect
--
critical
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: MarcoZ, Assigned: tbsaunde)

Tracking

({crash, regression})

5 Branch
mozilla6
x86
Windows NT
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox5+ fixed)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-e221982b-eede-4e8d-9ed1-48ab62110413 .
============================================================= 
This is another crash possibly regressed from bug 634218 which I currently can only reproduce in one specific textarea, but in that, I can repro it 100% of the time.
Component: Disability Access → Disability Access APIs
Product: Firefox → Core
QA Contact: disability.access → accessibility-apis
This is a regression that was introduced in the code that went into Aurora.
Assignee

Comment 2

8 years ago
So, looking at the stack mContent must be null which means the accessible is almost certainly defunct.  We got into NativeState() from nsHTMLTextFieldAccessible::GetValue()   .  So GetValue() is being called on a defunct accessible.  So the question is where should we check to see if the object is defunct.
Assignee

Comment 3

8 years ago
Posted patch test patchSplinter Review
I suspect there are a couple other place we call NativeState() not using State() that we might not check if the  o object is defunct.  I'l try to check for these and have a better patch in a bit.  This patch should solve the reported issue though.

 We could change internal calls to use State(), but I'm not sure if there's a reason we didn't do that before.
Assignee

Updated

8 years ago
Attachment #525656 - Flags: review?(surkov.alexander)
Attachment #525656 - Flags: review?(mar.marcoz)
Attachment #525656 - Flags: review?(bolterbugz)
Comment on attachment 525656 [details] [diff] [review]
test patch

r=me since this definitely fixes the bug.
Attachment #525656 - Flags: review?(mar.marcoz) → review+

Comment 6

8 years ago
Might it be simpler to back out the original patch for Aurora, since it apparently caused several crash regressions?
(In reply to comment #6)
> Might it be simpler to back out the original patch for Aurora, since it
> apparently caused several crash regressions?

I think generally yes, but in this case no.
(In reply to comment #3)
> Created attachment 525656 [details] [diff] [review]
> test patch
> 
> I suspect there are a couple other place we call NativeState() not using
> State() that we might not check if the  o object is defunct.  I'l try to check
> for these and have a better patch in a bit.

Trevor did you investigate this?
OK please ignore comment 7.

Given the spirit of our new development process, and a risk/reward analysis, I think we should resolve this and related fallout via backing out bug 634218 for FF5.

Aside: Trevor is aware and agrees via IRC
Comment on attachment 525656 [details] [diff] [review]
test patch

r=me, but why would you need so many reviews?
Attachment #525656 - Flags: review?(surkov.alexander) → review+
Assignee

Comment 11

8 years ago
(In reply to comment #8)
> (In reply to comment #3)
> > Created attachment 525656 [details] [diff] [review]
> > test patch
> > 
> > I suspect there are a couple other place we call NativeState() not using
> > State() that we might not check if the  o object is defunct.  I'l try to check
> > for these and have a better patch in a bit.
> 
> Trevor did you investigate this?

yes, so there are two other places that might have simlar issues the xul and html checkbox implementations of GetActionName() they currently use NativeState(), one fix for this would be to use State() instead of NativeState().  However I'm not sure if that would create a correctness issue where ARIA is used where it shouldn't be or if it fixes one using it where it previously wasn't but should have been.
I'd say it's worth to discuss these in other bug since neither of these can lead to crash.
Assignee

Updated

8 years ago
Attachment #525656 - Flags: review?(bolterbugz) → review?
Assignee

Updated

8 years ago
Attachment #525656 - Flags: review?
landed on m-c - http://hg.mozilla.org/mozilla-central/rev/987aaff80567
Assignee: nobody → trev.saunders
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110425 Firefox/6.0a1
Status: RESOLVED → VERIFIED
This is needed on aurora (or beta) 5, is the landed patch appropriate for Firefox 5 as well? If so, please request approval for the patch.
Assignee

Comment 16

8 years ago
(In reply to comment #15)
> This is needed on aurora (or beta) 5, is the landed patch appropriate for
> Firefox 5 as well? If so, please request approval for the patch.

hu?  this seems really odd... We backed 634218 out on aurora so I'm not really sure what's going on here, can I see a stack or something?

If its actually this issue I could probably come up with a null check and assert patch quickly, but because of the backout of 634218 I doubt any current patch would be suitable for aurora.

thanks!
HU? THAT SOUNDS REALLY ODD SINCE
Right, JST, this was a regression from bug 634218, and that was backed out from Aurora shortly after the initial merge, along with a few followups that had already landed to fix crashes introduced by that bug.
Just mark it fixed and move on, then ;-)
Crash Signature: [@ nsAccessible::NativeState()]
You need to log in before you can comment on or make changes to this bug.