Closed Bug 649604 Opened 10 years ago Closed 10 years ago

crash [@ nsAccessible::NativeState()]

Categories

(Core :: Disability Access APIs, defect)

5 Branch
x86
Windows NT
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla6
Tracking Status
firefox5 + fixed

People

(Reporter: MarcoZ, Assigned: tbsaunde)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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.
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.
Attached 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.
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+
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+
(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.
Attachment #525656 - Flags: review?(bolterbugz) → review?
Attachment #525656 - Flags: review?
landed on m-c - http://hg.mozilla.org/mozilla-central/rev/987aaff80567
Assignee: nobody → trev.saunders
Status: NEW → RESOLVED
Closed: 10 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.
(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.