Bug 210170 – ASSERTION FAILED: selection.isRange() in InsertListCommand::doApply

• Component: HTML Editing
• Status: RESOLVED FIXED

[Jack]

ASSERTION FAILED: selection.isRange()
./editing/InsertListCommand.cpp(138) : virtual void WebCore::InsertListCommand::doApply()
1   0x14b9ac639 WTFCrash
2   0x122494150 PAL::SOAuthorizationHintsFunction()
3   0x12931161f WebCore::InsertListCommand::doApply()
4   0x1291bfc5b WebCore::CompositeEditCommand::apply()
5   0x1292e81a8 WebCore::executeInsertUnorderedList(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&)
6   0x12927c105 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const
7   0x128bf577a WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&)
8   0x123b24782 WebCore::jsDocumentPrototypeFunctionExecCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*, JSC::ThrowScope&)
9   0x1237fce2e long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunctionExecCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)
10  0x1237fc984 WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::JSGlobalObject*, JSC::CallFrame*)
11  0x52978d801178
12  0x14ca0f31b llint_entry
13  0x14ca0f31b llint_entry
14  0x14c9f1ef2 vmEntryToJavaScript
15  0x14f01e630 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
16  0x14f01f8ed JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
17  0x14f8ddb79 JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
18  0x14f8de0bd JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
19  0x14f8dee03 JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
20  0x127f06427 WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
21  0x127f54809 WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&)
22  0x128e3c14b WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::DumbPtrTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>, WebCore::EventTarget::EventInvokePhase)
23  0x128e32b95 WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase)
24  0x128f3bc26 WebCore::Node::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase)
25  0x128e0974f WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const
26  0x128e09ad4 WebCore::MouseOrFocusEventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const
27  0x128e0ac3e WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&)
28  0x128e0a47b WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&)
29  0x128f3bc7d WebCore::Node::dispatchEvent(WebCore::Event&)
30  0x128d8ea03 WebCore::Element::dispatchFocusEvent(WTF::RefPtr<WebCore::Element, WTF::DumbPtrTraits<WebCore::Element> >&&, WebCore::FocusDirection)
31  0x12985e379 WebCore::HTMLSelectElement::dispatchFocusEvent(WTF::RefPtr<WebCore::Element, WTF::DumbPtrTraits<WebCore::Element> >&&, WebCore::FocusDirection)

[Jack]

<rdar://problem/61410397>

[Jack]

Created attachment 395771 [details]
Patch

[Jack]

Root cause for the assertion crash:
1. The code is executing command “insertUnorderedList”.
2. Because the original selected range for insertion is inside the table, in function selectionForParagraphIteration we try to move the position to the end of the table.
3. The end happens to be a text node, so the returned selection becomes “CaretSelection” and fails the assertion.

BODY	0x60c00006c280 (renderer 0x6120000634c0) 
	LABEL	0x60c00006c340 (renderer 0x6110000c3cc0) 
		#text	0x60800004fea0 "\n"
		UL	0x60c00006c400 (renderer 0x612000063640) 
			#text	0x60800004ff20 "\n"
			SELECT	0x613000063a80 (renderer 0x615000083980) 
				#text	0x6080000540a0 "\n"
			IFRAME	0x613000063c40 (renderer 0x612000063dc0) 
*			TABLE	0x60e0000597c0 (renderer 0x61400003ae40) 
				#text	0x60800004ffa0 "\n"
				TBODY	0x60c00006c4c0 (renderer 0x61300006c680) 
					TR	0x60c00006c580 (renderer 0x6110000c4080) 
						TH	0x60c00006c640 (renderer 0x6120000640c0) 
							#text	0x608000054120 "a"
						#text	0x6080000541a0 "\n\n"
						STYLE	0x61000002dd40 (renderer 0x0) 
						#text	0x608000056ca0 "\n"
after, offset:0

[Jack]

Test case:

<script>
    window.onload = () => {
        window.getSelection().setBaseAndExtent(TH,1,STYLE,1);
        document.execCommand("insertUnorderedList", false);
    }
</script>
<body contenteditable="true"><table><th id=TH>a</th><style id=STYLE></style>

[Wenson Hsieh]

Comment on attachment 395771 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395771&action=review

> LayoutTests/editing/inserting/insert-list-in-table-assert.html:13
> +            if (window.testRunner) {

Nit - no braces around the single line if statement.

> LayoutTests/editing/inserting/insert-list-in-table-assert.html:15
> +            }

Ditto.

> LayoutTests/editing/inserting/insert-list-in-table-assert.html:19
> +<body contenteditable="true"><table><th id=TH>a</th><style id=STYLE></style>

Nit - let’s add some closing tags to clean up the markup a bit.

I’m also curious why (or if) the style element is needed in the repro case.

[Jack]

Created attachment 396115 [details]
Patch for landing

[Jack]

Thanks Wenson. As you mentioned, I tried replacing <style> with <p> or <span>, the issue doesn't reproduce. However, with <script> it crashes.

And sorry somehow land-safely didn't work, so could you please review again?

(In reply to Wenson Hsieh from comment #5)
> Comment on attachment 395771 [details]
> Patch
> 
> I’m also curious why (or if) the style element is needed in the repro case.

[Jack]

Wait, it seems to work. It is in the commit queue now.
(In reply to Jack from comment #7)
> And sorry somehow land-safely didn't work, so could you please review again?

[EWS]

Committed r259899: <https://trac.webkit.org/changeset/259899>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396115 [details].