The default bug view has changed. See this FAQ.

Crash [@ nsINode::GetOwnerDoc()] with getAccessibleFor(null)

RESOLVED FIXED in mozilla7

Status

()

Core
Disability Access APIs
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: tbsaunde)

Tracking

({crash, testcase})

Trunk
mozilla7
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 542107 [details]
testcase (uses enhanced privileges)

See testcase, which crashes current trunk build. The testcase uses enhanced privileges.

https://crash-stats.mozilla.com/report/index/bp-426175f8-445a-45bc-a46e-3c58d2110627
0 	xul.dll 	nsINode::GetOwnerDoc 	obj-firefox/dist/include/nsINode.h:415
1 	xul.dll 	nsAccessibilityService::GetAccessible 	accessible/src/base/nsAccessibilityService.cpp:848
2 	xul.dll 	nsAccessibilityService::GetAccessibleFor 	accessible/src/base/nsAccessibilityService.cpp:640
3 	xul.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
4 	xul.dll 	XPC_WN_CallMethod 	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1607
5 	mozjs.dll 	js::Invoke 	js/src/jsinterp.cpp:656
6 	mozjs.dll 	js::Interpret 	js/src/jsinterp.cpp:4550
7 	mozjs.dll 	JSCompartment::wrap 	js/src/jscompartment.cpp:224
8 	mozjs.dll 	js::ExternalExecute 	js/src/jsinterp.cpp:944
9 	mozjs.dll 	EvaluateUCScriptForPrincipalsCommon 	js/src/jsapi.cpp:4984
10 	mozjs.dll 	JS_EvaluateUCScriptForPrincipalsVersion 	js/src/jsapi.cpp:5000
11 	xul.dll 	nsJSContext::EvaluateString 	dom/base/nsJSEnvironment.cpp:1453
12 	xul.dll 	nsScriptLoader::EvaluateScript 	content/base/src/nsScriptLoader.cpp:906
13 	xul.dll 	nsScriptLoader::ProcessRequest 	content/base/src/nsScriptLoader.cpp:799
14 	xul.dll 	nsScriptLoader::ProcessScriptElement 	content/base/src/nsScriptLoader.cpp:745
15 	xul.dll 	nsScriptElement::MaybeProcessScript 	content/base/src/nsScriptElement.cpp:182
16 	xul.dll 	nsHTMLScriptElement::MaybeProcessScript 	content/html/content/src/nsHTMLScriptElement.cpp:586
17 	xul.dll 	nsHTMLScriptElement::DoneAddingChildren 	content/html/content/src/nsHTMLScriptElement.cpp:513
18 	xul.dll 	nsHtml5TreeOpExecutor::RunScript 	parser/html/nsHtml5TreeOpExecutor.cpp:730
19 	xul.dll 	nsHtml5TreeOpExecutor::RunFlushLoop 	parser/html/nsHtml5TreeOpExecutor.cpp:525
20 	xul.dll 	nsHtml5ExecutorFlusher::Run 	parser/html/nsHtml5StreamParser.cpp:156
etc..

Updated

6 years ago
Assignee: nobody → trev.saunders
(Assignee)

Comment 1

6 years ago
Created attachment 542377 [details] [diff] [review]
trivial patch

null check the node arg

Review: surkov

Comment 2

6 years ago
Comment on attachment 542377 [details] [diff] [review]
trivial patch

Review of attachment 542377 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comment addressed

::: accessible/src/base/nsAccessibilityService.cpp
@@ +639,1 @@
>  

I think we should fail peaceful. I don't think JS needs exception if null is passed. Null as accessible is suitable answer for null as DOM node.

How about

if (!aNode)
  return NS_OK;
Attachment #542377 - Flags: review+
(Assignee)

Comment 3

6 years ago
(In reply to comment #2)
> Comment on attachment 542377 [details] [diff] [review] [review]
> trivial patch
> 
> Review of attachment 542377 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> r=me with comment addressed
> 
> ::: accessible/src/base/nsAccessibilityService.cpp
> @@ +639,1 @@
> >  
> 
> I think we should fail peaceful. I don't think JS needs exception if null is
> passed. Null as accessible is suitable answer for null as DOM node.

ok, that doesn't seem to unreasonable to me.

we need to do *aAccessible = nsnull first otherwise we're returning  whatever random value was in that pointer, but  otherwise  I'm fine with this.

> if (!aNode)
>   return NS_OK;
(Assignee)

Comment 4

6 years ago
Created attachment 542631 [details] [diff] [review]
patch v2
Attachment #542377 - Attachment is obsolete: true

Comment 5

6 years ago
(In reply to comment #3)

> we need to do *aAccessible = nsnull first 

right, thank you
(Assignee)

Comment 6

6 years ago
landed http://hg.mozilla.org/mozilla-central/rev/98285b0ee9a1
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Target Milestone: --- → mozilla7
(Reporter)

Comment 7

6 years ago
Would this fix bug 667398 too?

Updated

6 years ago
Crash Signature: [@ nsINode::GetOwnerDoc() ]
You need to log in before you can comment on or make changes to this bug.