Closed
Bug 429654
Opened 16 years ago
Closed 16 years ago
Crash Crash [@ nsDocAccessible::CreateTextChangeEventForNode(nsIAccessible*, nsIDOMNode*, nsIAccessible*, int, int) ]
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9
People
(Reporter: MarcoZ, Assigned: surkov)
References
Details
(Keywords: access, crash)
Crash Data
Attachments
(1 file)
1.75 KB,
patch
|
aaronlev
:
review+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
Found while triaging 3.0b5 crash reports. One crash report: bp-08c7e926-0b1d-11dd-82e4-0013211cbf8a The line indicated is: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/accessible/src/base/nsDocAccessible.cpp&rev=1.238&mark=1387#1387 Looks like *aContainerAccessible is being used without being checked for not NSNULL first. May there be situations where aContainerAccessible may be NULL? Should there be a check here?
Flags: blocking1.9?
Comment 1•16 years ago
|
||
Not enough info to block. Would take a patch.
Flags: blocking1.9? → blocking1.9-
Assignee | ||
Comment 2•16 years ago
|
||
Marco, let's put an assertion and not call CreateTextChangeEventForNode though it's suspicious GetAccessibleInParentChain return nsnull.
Assignee | ||
Comment 3•16 years ago
|
||
I think it makes sense to follow to http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/accessible/src/base/nsDocAccessible.cpp&rev=1.238&mark=1864#1864
Assignee | ||
Comment 4•16 years ago
|
||
Attachment #316545 -
Flags: review?(aaronleventhal)
Comment 5•16 years ago
|
||
Should we put that logic into GetAccessibleInParentChain() ... basically make sure it never returns null and returns the doc accessible if necessary?
Updated•16 years ago
|
Attachment #316545 -
Flags: review?(aaronleventhal) → review+
Assignee | ||
Comment 6•16 years ago
|
||
(In reply to comment #5) > Should we put that logic into GetAccessibleInParentChain() ... basically make > sure it never returns null and returns the doc accessible if necessary? > good idea, I'll put new patch for this
Assignee | ||
Updated•16 years ago
|
Assignee: ginn.chen → surkov.alexander
Assignee | ||
Comment 7•16 years ago
|
||
(In reply to comment #6) > (In reply to comment #5) > > Should we put that logic into GetAccessibleInParentChain() ... basically make > > sure it never returns null and returns the doc accessible if necessary? > > > > good idea, I'll put new patch for this > Aaron, GetAccessibleInParentChain() is used in many places and nsnull result is processed in different ways. I wouldn't risk to change this in the meantime. So if you fine with this then let's save the current approach.
Assignee | ||
Updated•16 years ago
|
Attachment #316545 -
Flags: approval1.9?
Comment 8•16 years ago
|
||
Ok, thank you for looking at it anyway.
Comment 9•16 years ago
|
||
Possible to test? re-request approval once addressed.
Updated•16 years ago
|
Attachment #316545 -
Flags: approval1.9? → approval1.9-
Assignee | ||
Comment 10•16 years ago
|
||
Comment on attachment 316545 [details] [diff] [review] patch (In reply to comment #9) > Possible to test? re-request approval once addressed. > We haven't a way to reproduce the bug. We just have stack trace. I tried to do small changes I can. If the test is major condition for the current status of the tree then let's wait for 1.9.x to land this trying of the the crash fix. re-requesting approval
Attachment #316545 -
Flags: approval1.9- → approval1.9?
Comment 11•16 years ago
|
||
Comment on attachment 316545 [details] [diff] [review] patch What's the regression risk? Sorry to keep asking this, but we need to be extra extra careful right now. Re-nom when answered.
Attachment #316545 -
Flags: approval1.9? → approval1.9-
Assignee | ||
Comment 12•16 years ago
|
||
Comment on attachment 316545 [details] [diff] [review] patch (In reply to comment #11) > (From update of attachment 316545 [details] [diff] [review]) > What's the regression risk? Sorry to keep asking this, but we need to be extra > extra careful right now. Re-nom when answered. > Since we get a notification from layout module and we don't get which element has been changed then we must invalidate the document entirely. Here we do this by sending corresponding events to accessible for document node. I think the risk is minimal, it doesn't bring major changes. re-requesting approval
Attachment #316545 -
Flags: approval1.9- → approval1.9?
Comment 13•16 years ago
|
||
Comment on attachment 316545 [details] [diff] [review] patch a1.9=beltzner
Attachment #316545 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 14•16 years ago
|
||
/cvsroot/mozilla/accessible/src/base/nsDocAccessible.cpp,v <-- nsDocAccessible.cpp new revision: 1.241; previous revision: 1.240 done
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Crash Signature: [@ nsDocAccessible::CreateTextChangeEventForNode(nsIAccessible*, nsIDOMNode*, nsIAccessible*, int, int) ]
You need to log in
before you can comment on or make changes to this bug.
Description
•