348197 - firefox crash with orca running [@ nsAccessNode::GetDocAccessibleFor]

Status: RESOLVED FIXED

(Firefox :: Disability Access, defect)

Description

[Evan Yan]

this bug can be easily reproduced by the steps below.

stepsA:
1. start orca;
2. start firefox;

stepsB:
1. start firefox;
2. start orca;
3. select menu->Edit->Preference.

the error message is:

###!!! ASSERTION: This method currently only supports docshells: 'docShell', file ../../../../mozilla/accessible/src/base/nsAccessNode.cpp, line 513
###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../../dist/include/xpcom/nsCOMPtr.h, line 849
Segmentation Fault - core dumped

It seems a regression introduced by bug 344539

Comment 1

[Evan Yan]

Attachment: core dump stack

Comment 2

[Evan Yan]

Attachment: patch

add null check

Comment 3

[Aaron Leventhal]

Comment on attachment 233070 [details] [diff] [review]
patch

I'm not sure this is still needed after the checkin for bug 347426, but I'll r= it. Please check to see if that checkin already fixes this.

Comment 4

[Evan Yan]

(In reply to comment #3)
> I'm not sure this is still needed after the checkin for bug 347426, but I'll r=
> it. Please check to see if that checkin already fixes this.
> 
the build I worked on was checked out on 2006-08-09, later than the checkin for bug 347426. GetContentDocShell() still return null sometimes, so I think we need the null check.

Comment 5

[Aaron Leventhal]

When is sometimes?

Comment 6

[Evan Yan]

(In reply to comment #5)
> When is sometimes?
> 
I debugged under the condition as the bug description, there are two cases that GetContentDocShell() returns null.
one is "if (State(accessible) & STATE_INVISIBLE)" get true; the other is "treeNode->GetChildCount(&subDocuments)" get zero child.

Comment 7

[Aaron Leventhal]

Is it on both trunk and branch, or just one of those?

I want to avoid "wallpaper" fixes. That is the name for fixes where we don't understand the root cause of the null pointer. Sometimes if you investigate you find the bug is really bigger and somewhere else.

Comment 8

[Evan Yan]

It's only on trunk build.
on 1.8 branch, because the patch of bug 346858 didn't get into it, we don't deal with RELATION_EMBEDS in 1.8 branch, so the crash code will never be called.

the root cause of this bug:
nsRootAccessible doesn't have content area all the time.

GetContentDocShell() will return null in two cases as follows.
1. refRelationSetCB() may be called when the Document's state is STATE_INVISIBLE, like when firefox start up.
it will return null at http://lxr.mozilla.org/seamonkey/source/accessible/src/base/nsRootAccessible.cpp#942
2. when a dialog like prefPane pop up, the type of the docShell is nsIDocShellTreeItem::typeChrome, and it has no subDocument. nsRootAccessible also has no content area in this condition.

I think it's reasonable for GetContentDocShell() to return null in those conditions, so adding null check in the caller function nsRootAccessible::GetAccessibleRelated() is the right fix.

Aaron, how do you think?

Comment 9

[Aaron Leventhal]

Thanks for the explanation. Please add a comment next to the null check and land this fix! :)