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

RESOLVED FIXED

Status

()

Core
Disability Access APIs
--
critical
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: MarcoZ, Assigned: MarcoZ)

Tracking

({access, crash})

Trunk
x86
Windows XP
access, crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
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.

Updated

10 years ago
Assignee: nobody → surkov.alexander

Comment 1

10 years ago
fixed in bug 454997
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 2

10 years ago
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 → ---
(Assignee)

Comment 3

10 years ago
And this report, too:
bp-1d6cbd40-9827-11dd-998d-001a4bd43e5c
(Assignee)

Comment 4

10 years ago
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.

Comment 5

10 years ago
What version of Jaws build do you use? I can't reproduce the crash.

Comment 6

10 years ago
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.
(Assignee)

Comment 7

10 years ago
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.

Comment 8

10 years ago
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.
(Assignee)

Comment 9

10 years ago
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.
(Assignee)

Comment 10

10 years ago
Created attachment 345339 [details] [diff] [review]
Add null check

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 11

10 years ago
Comment on attachment 345339 [details] [diff] [review]
Add null check

The } needs to be moved back 2 spaces.
Attachment #345339 - Flags: review?(aaronleventhal) → review+

Comment 12

10 years ago
Marco, please add assertion.
(Assignee)

Comment 13

10 years ago
Pushed with Aaron's nit and Surkov's request in changeset:
http://hg.mozilla.org/mozilla-central/rev/4ea96420ec0f
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
Blocks: 462281
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?

Comment 15

9 years ago
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.