1403841 - Crash in nsINode::IsEditableInternal

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P1)

Description

[[:philipp]]

[Tracking Requested - why for this release]:
this crash signature is spiking up in 57.0b and 58.0a1. it's cross-platform and seems to hit particular users repeatedly (on b3 we have >1000 crashes from 90 installations right now).

This bug was filed from the Socorro interface and is 
report bp-2fd27fd8-0973-44f2-9aa2-561be0170927.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	nsINode::IsEditableInternal() 	dom/base/nsINode.cpp:216
1 	xul.dll 	mozilla::EditorBase::IsSelectionEditable() 	editor/libeditor/EditorBase.cpp:566
2 	xul.dll 	nsHighlightColorStateCommand::IsCommandEnabled(char const*, nsISupports*, bool*) 	editor/composer/nsComposerCommands.cpp:88
3 	xul.dll 	nsControllerCommandTable::IsCommandEnabled(char const*, nsISupports*, bool*) 	dom/commandhandler/nsControllerCommandTable.cpp:98
4 	xul.dll 	nsBaseCommandController::IsCommandEnabled(char const*, bool*) 	dom/commandhandler/nsBaseCommandController.cpp:105
5 	xul.dll 	nsWindowRoot::GetEnabledDisabledCommandsForControllers(nsIControllers*, nsTHashtable<nsCharPtrHashKey>&, nsTArray<nsTString<char> >&, nsTArray<nsTString<char> >&) 	dom/base/nsWindowRoot.cpp:329
6 	xul.dll 	nsWindowRoot::GetEnabledDisabledCommands(nsTArray<nsTString<char> >&, nsTArray<nsTString<char> >&) 	dom/base/nsWindowRoot.cpp:355
7 	xul.dll 	ChildCommandDispatcher::Run() 	dom/base/nsGlobalWindow.cpp:10153
8 	xul.dll 	nsContentUtils::AddScriptRunner(already_AddRefed<nsIRunnable>) 	dom/base/nsContentUtils.cpp:5712
9 	xul.dll 	nsContentUtils::AddScriptRunner(nsIRunnable*) 	dom/base/nsContentUtils.cpp:5719
10 	xul.dll 	nsGlobalWindow::UpdateCommands(nsTSubstring<char16_t> const&, nsISelection*, short) 	dom/base/nsGlobalWindow.cpp:10193
11 	xul.dll 	nsDocViewerSelectionListener::NotifySelectionChanged(nsIDOMDocument*, nsISelection*, short) 	layout/base/nsDocumentViewer.cpp:3844
12 	xul.dll 	mozilla::dom::Selection::NotifySelectionListeners() 	dom/base/Selection.cpp:3879
13 	xul.dll 	nsFrameSelection::NotifySelectionListeners(mozilla::SelectionType) 	layout/generic/nsFrameSelection.cpp:2053
14 	xul.dll 	mozilla::dom::Selection::RemoveRange(nsRange&, mozilla::ErrorResult&) 	dom/base/Selection.cpp:2519
15 	xul.dll 	mozilla::dom::Selection::RemoveRange(nsIDOMRange*) 	dom/base/Selection.cpp:2447
16 	xul.dll 	nsRange::SetSelection(mozilla::dom::Selection*) 	dom/base/nsRange.cpp:1061
17 	xul.dll 	mozilla::dom::Selection::AddItemInternal(nsRange*, int*) 	dom/base/Selection.cpp:1219
18 	xul.dll 	mozilla::dom::Selection::AddItem(nsRange*, int*, bool) 	dom/base/Selection.cpp:1203
19 	xul.dll 	mozilla::dom::Selection::GetTableCellLocationFromRange(nsRange*, int*, int*, int*) 	dom/base/Selection.cpp:592
20 	xul.dll 	mozilla::dom::Selection::AddRangeInternal(nsRange&, nsIDocument*, mozilla::ErrorResult&) 	dom/base/Selection.cpp:2394
21 	xul.dll 	mozilla::dom::Selection::AddRange(nsRange&, mozilla::ErrorResult&) 	dom/base/Selection.cpp:2360
22 	xul.dll 	mozilla::dom::SelectionBinding::addRange 	dom/bindings/SelectionBinding.cpp:294
23 	xul.dll 	mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) 	dom/bindings/BindingUtils.cpp:3055
24 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:495
25 	xul.dll 	InternalCall 	js/src/vm/Interpreter.cpp:540
26 	xul.dll 	js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) 	js/src/proxy/Wrapper.cpp:175
27 	xul.dll 	js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) 	js/src/proxy/CrossCompartmentWrapper.cpp:359
28 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:477
29 	xul.dll 	InternalCall 	js/src/vm/Interpreter.cpp:540
30 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp:3084
31 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:435
32 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:513
33 	xul.dll 	InternalCall 	js/src/vm/Interpreter.cpp:540
34 	xul.dll 	js::fun_apply(JSContext*, unsigned int, JS::Value*) 	js/src/jsfun.cpp:1302
35 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:495
36 	xul.dll 	InternalCall 	js/src/vm/Interpreter.cpp:540
37 	xul.dll 	js::jit::DoCallFallback 	js/src/jit/BaselineIC.cpp:2589
38 		@0xea67421 	
39 		@0x17c570f7 	
40 		@0xea6094e 	
41 	xul.dll 	js::jit::EnterBaselineMethod(JSContext*, js::RunState&) 	js/src/jit/BaselineJIT.cpp:200
42 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:425
43 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:513
44 	xul.dll 	js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp:559
45 	xul.dll 	JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) 	js/src/jsapi.cpp:2965
46 	xul.dll 	xpc::WrapperFactory::PrepareForWrapping(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::MutableHandle<JSObject*>) 	js/xpconnect/wrappers/WrapperFactory.cpp:211

Comment 1

[Makoto Kato [:m_kato]]

Attachment: Bug 1403841 - IsSelectionEditable should check whether focus node and anchor node aren't null.

This might be regression by bug 1319340, but there is crash signature from old version.  We should check whether focus node and anchor node aren't null.

Review commit: https://reviewboard.mozilla.org/r/184932/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184932/

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8913552 [details]
Bug 1403841 - IsSelectionEditable should check whether focus node and anchor node aren't null.

https://reviewboard.mozilla.org/r/184932/#review190024

::: commit-message-e6c32:3
(Diff revision 1)
> +Bug 1403841 - IsSelectionEditable should check whether focus node and anchor node aren't null. r?masayuki
> +
> +This might be regression by bug 1319340, but there is crash signature from old version.  We should check whether focus node and anchor node aren't null.

Hmm, really odd...

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33868c610ffc
IsSelectionEditable should check whether focus node and anchor node aren't null. r=masayuki

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/33868c610ffc

Comment 5

[Sylvestre Ledru [:Sylvestre]]

I guess we would like to uplift that to 57?

Comment 6

[Makoto Kato [:m_kato]]

Comment on attachment 8913552 [details]
Bug 1403841 - IsSelectionEditable should check whether focus node and anchor node aren't null.

Approval Request Comment
[Feature/Bug causing the regression]:
Possible bug 1319340

[User impact if declined]:
Firefox crashes when Selection.AddRange etc is called by user script

[Is this code covered by automated tests?]:
No.  I don't know how to reproduce this.

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]: 
No, I don't know how to reproduce this.

[List of other uplifts needed for the feature/fix]:
No

[Is the change risky?]:
No

[Why is the change risky/not risky?]:
null check only

[String changes made/needed]:
None.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8913552 [details]
Bug 1403841 - IsSelectionEditable should check whether focus node and anchor node aren't null.

Fix a crash, taking it.
Should be in 57b5

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/274c04f3c565