Closed Bug 458058 Opened 12 years ago Closed 12 years ago

Crash [@ CallQueryInterface<nsINode, nsIDOMElement>(nsINode*, nsIDOMElement**) ]

Categories

(Core :: Disability Access APIs, defect, critical)

x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: MarcoZ, Assigned: MarcoZ)

References

()

Details

(Keywords: access, crash)

Crash Data

Attachments

(1 file)

I ran into this at least twice a day over the past 5 days or so. Reports are:
bp-4d928dd9-8f99-11dd-a004-001a4bd43ed6
bp-0179e554-8eb8-11dd-9bfd-001cc4e2bf68

They both point at a line in nsAccUtils::GetDomElementFor. And they both come from nsAccessNode::GetComputedStyleDeclaration.

Observation: I always used JAWS 10 public beta 2 (build 10.0.396) when these crashes occurred.

Alex, since these both point at a line near to where crashes after fix for bug 454997 also pointed to, these might be related.
Assignee: nobody → surkov.alexander
fixed in bug 454997
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Still seeing crashes in this area like
bp-d57a883d-97b5-11dd-b5bc-0013211cbf8a

in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081011 Minefield/3.1b2pre
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
And this report, too:
bp-1d6cbd40-9827-11dd-998d-001a4bd43e5c
Most reliable way to reproduce:
1. Have JAWS 10 running.
2. Go to twitter.com. I have it set to automatically log in, so I immediately get to the page where I can enter my update.
3. Often, when the page comes up, Minefield will crash with stack traces like the two quoted in the previous comments.
What version of Jaws build do you use? I can't reproduce the crash.
One idea I have is text node has been removed but we didn't invalidated the cache. Possibly related with nsAccessNode caching if text node is hidden.
New crashes with 20081029 build:
bp-d6d08997-a5c7-11dd-8d28-001a4bd43ef6 and similar. All these were captured from doing this:

1. With JAWS 10 public beta 3 or later running, go to http://www.nvda-project.org/ticket/215.
2. Log in (or create an account if you don't have one, then log in).
3. Once the page is reloaded with the additional possibilities of a logged-in user, Firefox crashes.
Any ticket such as http://www.nvda-project.org/ticket/138 will also cause these crashes if you checked the box to automatically log in.
Signature	CallQueryInterface<nsIDOMHTMLElement, nsIDOMElement>(nsIDOMHTMLElement*, nsIDOMElement**)
UUID	d6d08997-a5c7-11dd-8d28-001a4bd43ef6
Time	2008-10-29 07:41:56-07
Uptime	119
Product	Firefox
Version	3.1b2pre
Build ID	20081029033642
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
CPU	x86
CPU Info	GenuineIntel family 6 model 15 stepping 11
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x0
Comments	
Crashing Thread
Frame 	Module 	Signature 	Source
0 	xul.dll 	CallQueryInterface<nsIDOMHTMLElement,nsIDOMElement> 	obj-firefox/dist/include/xpcom/nsISupportsUtils.h:203
1 	xul.dll 	nsCoreUtils::GetDOMElementFor 	accessible/src/base/nsCoreUtils.cpp:171
2 	xul.dll 	nsCoreUtils::GetComputedStyleDeclaration 	accessible/src/base/nsCoreUtils.cpp:724
3 	xul.dll 	nsAccessNodeWrap::get_computedStyleForProperties 	accessible/src/msaa/nsAccessNodeWrap.cpp:356

the crash address of 0x0 means that:

   171    CallQueryInterface(node->GetNodeParent(), &element);

crashed because node->GetNodeParent() returned null.
Timeless, I know. The question is how do we fix it. Do we simply wall-paper fix it by doing a NULL check, or do we find out the root cause for this and make sure every text node has a parent? Because that's what this says: A text node has no parent, which is close to impossible unless the cache is not invalidated correctly. I can put together a wallpaper fix in 2 minutes, but this doesn't cure the root cause.
Attached patch Add null checkSplinter Review
Check if the parent is valid before doing CallQueryInterface. Fixes the crashes.
Assignee: surkov.alexander → marco.zehe
Status: REOPENED → ASSIGNED
Attachment #345339 - Flags: review?(aaronleventhal)
Comment on attachment 345339 [details] [diff] [review]
Add null check

The } needs to be moved back 2 spaces.
Attachment #345339 - Flags: review?(aaronleventhal) → review+
Marco, please add assertion.
Pushed with Aaron's nit and Surkov's request in changeset:
http://hg.mozilla.org/mozilla-central/rev/4ea96420ec0f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
No longer blocks: 462281
A guy on IRC is complaining of a crash on paypal.

The stack is http://crash-stats.mozilla.com/report/index/169c0eb3-0331-46e9-a7a2-d9ab82090214?p=1

Frame  	Module  	Signature [Expand]  	Source
0 	xul.dll 	nsAccUtils::GetDOMElementFor 	mozilla/accessible/src/base/nsAccessibilityUtils.cpp:1115
1 	xul.dll 	nsAccessNode::GetComputedStyleDeclaration 	mozilla/accessible/src/base/nsAccessNode.cpp:645
2 	xul.dll 	nsHTMLTableAccessible::IsProbablyForLayout 	mozilla/accessible/src/html/nsHTMLTableAccessible.cpp:1144
3 	xul.dll 	nsHTMLTableAccessible::GetAttributesInternal 	mozilla/accessible/src/html/nsHTMLTableAccessible.cpp:233
4 	xul.dll 	nsCOMPtr<nsIPersistentProperties>::operator= 	nsCOMPtr.h:780
5 	xul.dll 	nsAccessible::GetAttributes 	mozilla/accessible/src/base/nsAccessible.cpp:2064
6 	xul.dll 	nsAccessibleWrap::get_attributes 	mozilla/accessible/src/msaa/nsAccessibleWrap.cpp:1580
7 	FSDomNodeFirefox.DLL 	FSDomNodeFirefox.DLL@0x3f49 	

1) Is this crash covered by this bug?
2) Is this fix in the 1.9.1 branch?
It isnt' seemed to me patch of the bug address problem shown by this stack trace. I think we might want to put null check into start of method nsAccUtils::GetDOMElementFor(nsIDOMNode *aNode) and put an assertion or precondition to notify about incorrect usage of the method.
Crash Signature: [@ CallQueryInterface<nsINode, nsIDOMElement>(nsINode*, nsIDOMElement**) ]
You need to log in before you can comment on or make changes to this bug.