Bug 224977

Summary:

Crash in InsertParagraphSeparatorCommand::doApply

Product:

WebKit

Reporter:

Ali Juma <ajuma>

Component:

HTML Editing

Assignee:

Frédéric Wang (:fredw) <fred.wang>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cdumez, cgarcia, esprehn+autocc, ews-feeder, ews-watchlist, fred.wang, gpoo, iang, kangil.han, mifenton, product-security, rbuis, rniwa, rohitrao, svillar, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=225267

Bug Depends on:

226527

Bug Blocks:

Attachments:

Description	Flags
Minimal test case	none
Reduction (ASSERTION FAILED: refNode) -- fixed in bug 226527	none
tentative patch for attachment 429009	ews-feeder: commit-queue-
Reduce a bit the second issue	none
Reduction (ASSERTION FAILED: startBlock->firstChild())	none
Add an assert for Position's offset	ews-feeder: commit-queue-
Add an assert for Position's offset	none
Patch for ASSERTION FAILED: startBlock->firstChild()	ews-feeder: commit-queue-
Patch for ASSERTION FAILED: refNode	rniwa: review+
Patch for ASSERTION FAILED: startBlock->firstChild()	ews-feeder: commit-queue-
Patch (fix offset)	ews-feeder: commit-queue-
Tentative patch	none
Patch	none
Patch (attempt to rebuild the dom tree and selection before crash)	none

Ali Juma

Reported 2021-04-23 07:49:17 PDT

Created attachment 426906 [details] Minimal test case Filing this as a security bug since it was found using a fuzzer; there's no disclosure deadline for this bug. This reproduces in an ASan build of WebKitTestRunner, and also in STP 123. Stack: ================================================================= ==35165==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x0006500bb361 bp 0x7ffee22d3cf0 sp 0x7ffee22d3cf0 T0) ==35165==The signal is caused by a READ memory access. ==35165==Hint: address points to the zero page. #0 0x6500bb360 in WebCore::Node::parentNode() const (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x18d360) #1 0x653784827 in WebCore::CompositeEditCommand::insertNodeBefore(WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >&&, WebCore::Node&, WebCore::ShouldAssumeContentIsAlwaysEditable) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3856827) #2 0x6538413ae in WebCore::InsertParagraphSeparatorCommand::doApply() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x39133ae) #3 0x65378c29a in WebCore::CompositeEditCommand::applyCommandToComposite(WTF::Ref<WebCore::EditCommand, WTF::RawPtrTraits<WebCore::EditCommand> >&&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x385e29a) #4 0x6538a5c25 in WebCore::TypingCommand::insertParagraphSeparator() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3977c25) #5 0x6538a3b07 in WebCore::TypingCommand::insertParagraphSeparatorAndNotifyAccessibility() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3975b07) #6 0x65376dba4 in WebCore::CompositeEditCommand::apply() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x383fba4) #7 0x6538a397a in WebCore::TypingCommand::insertParagraphSeparator(WebCore::Document&, unsigned int) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x397597a) #8 0x6538267a2 in WebCore::executeInsertParagraph(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x38f87a2) #9 0x65348cfa3 in WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x355efa3) #10 0x6509104aa in WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x9e24aa) #11 0x65090ff6b in long long WebCore::IDLOperation<WebCore::JSDocument>::call<&(WebCore::jsDocumentPrototypeFunction_execCommandBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSDocument*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x9e1f6b) #12 0x26ee116011d7 (<unknown module>) #13 0x66c9f826a in llint_entry (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb5026a) #14 0x66c9dd308 in vmEntryToJavaScript (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb35308) #15 0x66e1822fd in JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x22da2fd) #16 0x66e825d8f in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x297dd8f) #17 0x66e82614b in JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x297e14b) #18 0x652ce28b8 in WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2db48b8) #19 0x652d0b9d7 in WebCore::JSEventListener::handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2ddd9d7) #20 0x6535b0a5f in WebCore::EventTarget::innerInvokeEventListeners(WebCore::Event&, WTF::Vector<WTF::RefPtr<WebCore::RegisteredEventListener, WTF::RawPtrTraits<WebCore::RegisteredEventListener>, WTF::DefaultRefDerefTraits<WebCore::RegisteredEventListener> >, 1ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>, WebCore::EventTarget::EventInvokePhase) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3682a5f) #21 0x6535b0302 in WebCore::EventTarget::fireEventListeners(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3682302) #22 0x65357f349 in WebCore::EventContext::handleLocalEvents(WebCore::Event&, WebCore::EventTarget::EventInvokePhase) const (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3651349) #23 0x6535807ac in WebCore::dispatchEventInDOM(WebCore::Event&, WebCore::EventPath const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x36527ac) #24 0x65357fdb3 in WebCore::EventDispatcher::dispatchEvent(WebCore::Node&, WebCore::Event&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3651db3) #25 0x65368adf7 in WebCore::ScopedEventQueue::enqueueEvent(WTF::Ref<WebCore::Event, WTF::RawPtrTraits<WebCore::Event> >&&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x375cdf7) #26 0x65357f678 in WebCore::EventDispatcher::dispatchScopedEvent(WebCore::Node&, WebCore::Event&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x3651678) #27 0x65340cd9e in WebCore::dispatchChildRemovalEvents(WTF::Ref<WebCore::Node, WTF::RawPtrTraits<WebCore::Node> >&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x34ded9e) #28 0x6533f9b65 in WebCore::ContainerNode::removeChild(WebCore::Node&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x34cbb65) #29 0x6533fe07a in WebCore::ContainerNode::replaceChild(WebCore::Node&, WebCore::Node&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x34d007a) #30 0x65357073e in WebCore::Element::setOuterHTML(WTF::String const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x364273e) #31 0x6509d1746 in WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue)::'lambda'()::operator()() const (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0xaa3746) #32 0x6509d15e6 in void WebCore::invokeFunctorPropagatingExceptionIfNecessary<WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue)::'lambda'()>(JSC::JSGlobalObject&, JSC::ThrowScope&, WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue)::'lambda'()&&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0xaa35e6) #33 0x6509d1340 in WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0xaa3340) #34 0x65088d0d6 in bool WebCore::IDLAttribute<WebCore::JSElement>::set<&(WebCore::setJSElement_outerHTMLSetter(JSC::JSGlobalObject&, WebCore::JSElement&, JSC::JSValue)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, long long, long long, JSC::PropertyName) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x95f0d6) #35 0x66e8d4afa in JSC::callCustomSetter(JSC::JSGlobalObject*, bool (*)(JSC::JSGlobalObject*, long long, long long, JSC::PropertyName), bool, JSC::JSObject*, JSC::JSValue, JSC::JSValue, JSC::PropertyName) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x2a2cafa) #36 0x66eb4d5d6 in JSC::JSObject::putInlineSlow(JSC::JSGlobalObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x2ca55d6) #37 0x66e45364c in llint_slow_path_put_by_id (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x25ab64c) #38 0x66c9e752f in llint_entry (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb3f52f) #39 0x66c9dd308 in vmEntryToJavaScript (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0xb35308) #40 0x66e1822fd in JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x22da2fd) #41 0x66e825d8f in JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x297dd8f) #42 0x66e82614b in JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/JavaScriptCore.framework/Versions/A/JavaScriptCore:x86_64+0x297e14b) #43 0x652ce28b8 in WebCore::JSExecState::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2db48b8) #44 0x652dba67d in WebCore::ScheduledAction::executeFunctionInContext(JSC::JSGlobalObject*, JSC::JSValue, WebCore::ScriptExecutionContext&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2e8c67d) #45 0x652dba05a in WebCore::ScheduledAction::execute(WebCore::Document&) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x2e8c05a) #46 0x6544bf035 in WebCore::DOMTimer::fired() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x4591035) #47 0x65483bcb4 in WebCore::ThreadTimers::sharedTimerFiredInternal() (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x490dcb4) #48 0x6548c5939 in WebCore::timerFired(__CFRunLoopTimer*, void*) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebCore.framework/Versions/A/WebCore:x86_64+0x4997939) #49 0x7fff30b00467 in __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9f467) #50 0x7fff30afffcd in __CFRunLoopDoTimer (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9efcd) #51 0x7fff30affab8 in __CFRunLoopDoTimers (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x9eab8) #52 0x7fff30ae470c in __CFRunLoopRun (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x8370c) #53 0x7fff30ae3952 in CFRunLoopRunSpecific (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:x86_64+0x82952) #54 0x7fff331a11c7 in -[NSRunLoop(NSRunLoop) runMode:beforeDate:] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation:x86_64+0x601c7) #55 0x7fff33253c6e in -[NSRunLoop(NSRunLoop) run] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation:x86_64+0x112c6e) #56 0x7fff6acc04e9 in _xpc_objc_main.cold.4 (/usr/lib/system/libxpc.dylib:x86_64+0x164e9) #57 0x7fff6acc042f in _xpc_objc_main (/usr/lib/system/libxpc.dylib:x86_64+0x1642f) #58 0x7fff6acbff62 in xpc_main (/usr/lib/system/libxpc.dylib:x86_64+0x15f62) #59 0x640f03a83 in WebKit::XPCServiceMain(int, char const**) (/Users/chrome-bot/clusterfuzz/bot/builds/chrome-ios-webkit-to-fuzz_ios-webkit-to-fuzz_cb292771138f3c7c4bb12f2df778e2b1c42b4cd7/revisions/WebKitMacOS/WebKit.framework/Versions/A/WebKit:x86_64+0xf03a83) #60 0x7fff6aa6ecc8 in start (/usr/lib/system/libdyld.dylib:x86_64+0x1acc8) ==35165==Register values: rax = 0x0000000000000004 rbx = 0x00007ffee22d3d80 rcx = 0x0000100000000004 rdx = 0x0000000000000000 rdi = 0x0000000000000020 rsi = 0x00007ffee22d4370 rbp = 0x00007ffee22d3cf0 rsp = 0x00007ffee22d3cf0 r8 = 0x0000000000000001 r9 = 0x0000000000000000 r10 = 0xffffffffffffffff r11 = 0xfffffffffffffaf0 r12 = 0x00007ffee22d3d20 r13 = 0x00007ffee22d3d00 r14 = 0x00001fffdc45a7a0 r15 = 0x0000000000000000 ======================== Clusterfuzz-id: 5684221868572672

Attachments
Minimal test case (3.97 KB, text/html) 2021-04-23 07:49 PDT, Ali Juma	no flags	Details
Reduction (ASSERTION FAILED: refNode) -- fixed in bug 226527 (361 bytes, text/html) 2021-05-18 17:45 PDT, Ryosuke Niwa	no flags	Details
tentative patch for attachment 429009 (2.69 KB, patch) 2021-05-24 08:31 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details Formatted Diff Diff
Reduce a bit the second issue (2.10 KB, text/html) 2021-05-25 10:24 PDT, Frédéric Wang (:fredw)	no flags	Details
Reduction (ASSERTION FAILED: startBlock->firstChild()) (1.81 KB, text/html) 2021-05-26 03:31 PDT, Frédéric Wang (:fredw)	no flags	Details
Add an assert for Position's offset (535 bytes, patch) 2021-05-27 04:36 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details Formatted Diff Diff
Add an assert for Position's offset (537 bytes, patch) 2021-05-27 05:56 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch for ASSERTION FAILED: startBlock->firstChild() (8.74 KB, patch) 2021-06-02 06:31 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch for ASSERTION FAILED: refNode (4.57 KB, patch) 2021-06-02 06:47 PDT, Frédéric Wang (:fredw)	rniwa: review+	Details Formatted Diff Diff
Patch for ASSERTION FAILED: startBlock->firstChild() (11.75 KB, patch) 2021-06-09 05:47 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (fix offset) (7.67 KB, patch) 2021-06-15 02:41 PDT, Frédéric Wang (:fredw)	ews-feeder: commit-queue-	Details Formatted Diff Diff
Tentative patch (961 bytes, patch) 2021-06-15 06:12 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch (6.13 KB, patch) 2021-06-16 06:53 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch (attempt to rebuild the dom tree and selection before crash) (3.46 KB, patch) 2021-07-03 06:12 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2021-04-29 10:05:21 PDT

<rdar://problem/77327339>

Ryosuke Niwa

Comment 2 2021-05-18 17:44:51 PDT

<rdar://77270374>

Ryosuke Niwa

Comment 3 2021-05-18 17:45:35 PDT

Created attachment 429009 [details] Reduction (ASSERTION FAILED: refNode) -- fixed in bug 226527

Frédéric Wang (:fredw)

Comment 4 2021-05-22 12:29:11 PDT

Quick reverse-debugging shows we are in a situation where a <body> has display: table and a single <img> child. Then ContainerNode::traverseToChildAt with child index = 1 returns nullptr. Although the code involved is different from bug 225267, this issue is also prevented by attachment 428919 [details]. ASSERTION FAILED: refNode at https://webkit-search.igalia.com/webkit/rev/61241a69457b1d5d1c68c82b673f489e8e3caf81/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp#278 (rr) reverse-f (rr) reverse-f (rr) reverse-f (rr) p refNode $1 = (WebCore::Node *) 0x0 (rr) rn (rr) rn https://webkit-search.igalia.com/webkit/rev/61241a69457b1d5d1c68c82b673f489e8e3caf81/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp#277 (rr) p startBlock (rr) p showTree((Node*) startBlock) *BODY 0x7f92618707b0 (renderer 0x7f9261870a80) STYLE=display: table IMG 0x7f9261870840 (renderer 0x7f9261870c50) STYLE=content: '' $2 = void (rr) p insertionPosition.computeOffsetInContainerNode() $3 = 1

Frédéric Wang (:fredw)

Comment 5 2021-05-24 02:26:47 PDT

(In reply to Ryosuke Niwa from comment #3) > Created attachment 429009 [details] > Reduction I'm no longer able to reproduce the crash with this reduction. I'm also getting a new debug assert with the original testcase: ASSERTION FAILED: startBlock->firstChild() ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp(275) : virtual void WebCore::InsertParagraphSeparatorCommand::doApply()

Frédéric Wang (:fredw)

Comment 6 2021-05-24 08:31:25 PDT

Created attachment 429534 [details] tentative patch for attachment 429009 [details]

Frédéric Wang (:fredw)

Comment 7 2021-05-24 08:41:27 PDT

(In reply to Frédéric Wang (:fredw) from comment #5) > (In reply to Ryosuke Niwa from comment #3) > > Created attachment 429009 [details] > > Reduction > > I'm no longer able to reproduce the crash with this reduction. So there was some subtle thing happening. My editor adds a new line at the end of the tescase, which makes it no longer crash. Anyway, I uploaded a tentative patch to fix attachment 429009 [details], which is essentially the same I had tried for bug 225267 and bug 221387 (tweaking createFragmentFromText does not seem enough here). > I'm also getting a new debug assert with the original testcase: > > ASSERTION FAILED: startBlock->firstChild() > ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp(275) : > virtual void WebCore::InsertParagraphSeparatorCommand::doApply() This still happens with attachment 429534 [details] applied and we have a AddressSanitizer: SEGV. Still need to debug this one...

Frédéric Wang (:fredw)

Comment 8 2021-05-25 10:24:01 PDT

Created attachment 429664 [details] Reduce a bit the second issue Reducing a bit the issue from comment 7.

Frédéric Wang (:fredw)

Comment 9 2021-05-26 03:31:58 PDT

Created attachment 429744 [details] Reduction (ASSERTION FAILED: startBlock->firstChild())

Frédéric Wang (:fredw)

Comment 10 2021-05-26 10:53:10 PDT

(In reply to Frédéric Wang (:fredw) from comment #5) > (In reply to Ryosuke Niwa from comment #3) > > Created attachment 429009 [details] > > Reduction > > I'm no longer able to reproduce the crash with this reduction. I'm also > getting a new debug assert with the original testcase: > > ASSERTION FAILED: startBlock->firstChild() > ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp(275) : > virtual void WebCore::InsertParagraphSeparatorCommand::doApply() Preliminary debugging. At https://webkit-search.igalia.com/webkit/rev/59144827bcba50f5e287f957c38137c3bbe4a98e/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp#201 the state looks like this: (rr) p showTree(visiblePos) #document 0x61f00001dc80 (renderer 0x6160003ced80) html 0x60c0002a6a00 (renderer (nil)) (needs style recalc) HTML 0x60c0002a6ac0 (renderer 0x61200007da40) HTML 0x60c0002ab5c0 (renderer 0x61200006c4c0) * #text 0x60b0000e91f0 "0" DIV 0x60c0002aacc0 (renderer 0x61200006c640) HTML 0x60c0002aab40 (renderer 0x61200006c7c0) HTML 0x60c0002b1d40 (renderer 0x6120000a1440) legacy, offset, offset:0 $1 = void (rr) p isFirstInBlock $2 = true (rr) p isLastInBlock $3 = false When we hit the assert, startBlock is the inner (empty) HTML element, and so we are not following the comment saying "startBlock should always have children, otherwise isLastInBlock would be true and it's handled above.". The reason why isFirstInBlock is true is because PositionIterator::isCandidate() skips HTML element. Here is the backtrace: #0 WebCore::PositionIterator::isCandidate() const (this=0x7ffd3ab7fe60) at ../../Source/WebCore/dom/PositionIterator.cpp:176 #1 0x00007fc13932439e in WebCore::nextCandidate(WebCore::Position const&) (position=...) at ../../Source/WebCore/editing/Editing.cpp:198 #2 0x00007fc1394b5856 in WebCore::VisiblePosition::canonicalPosition(WebCore::Position const&) (passedPosition=...) at ../../Source/WebCore/editing/VisiblePosition.cpp:558 #3 0x00007fc1394aef6f in WebCore::VisiblePosition::VisiblePosition(WebCore::Position const&, WebCore::Affinity) (this=0x7ffd3ab80280, position=..., affinity=WebCore::Affinity::Downstream) at ../../Source/WebCore/editing/VisiblePosition.cpp:58 #4 0x00007fc1394da026 in WebCore::startOfBlock(WebCore::VisiblePosition const&, WebCore::EditingBoundaryCrossingRule) (visiblePosition=..., rule=WebCore::CanCrossEditingBoundary) at ../../Source/WebCore/editing/VisibleUnits.cpp:1338 #5 0x00007fc1394da7e2 in WebCore::isStartOfBlock(WebCore::VisiblePosition const&) (pos=...) at ../../Source/WebCore/editing/VisibleUnits.cpp:1357 #6 0x00007fc1393f6c63 in WebCore::InsertParagraphSeparatorCommand::doApply() (this=0x6120000a1740) at ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:200

Frédéric Wang (:fredw)

Comment 11 2021-05-27 04:36:11 PDT

Created attachment 429867 [details] Add an assert for Position's offset

Frédéric Wang (:fredw)

Comment 12 2021-05-27 05:56:06 PDT

Created attachment 429872 [details] Add an assert for Position's offset for EWS

Frédéric Wang (:fredw)

Comment 13 2021-05-27 06:56:34 PDT

(In reply to Frédéric Wang (:fredw) from comment #10) > (rr) p showTree(visiblePos) > #document 0x61f00001dc80 (renderer 0x6160003ced80) > html 0x60c0002a6a00 (renderer (nil)) (needs style recalc) > HTML 0x60c0002a6ac0 (renderer 0x61200007da40) > HTML 0x60c0002ab5c0 (renderer 0x61200006c4c0) > * #text 0x60b0000e91f0 "0" > DIV 0x60c0002aacc0 (renderer 0x61200006c640) > HTML 0x60c0002aab40 (renderer 0x61200006c7c0) > HTML 0x60c0002b1d40 (renderer 0x6120000a1440) So the fact that the startBlock is the inner HTML but the selection visiblePos on the text sibling is quite confusing. Debugging a bit more (see below), endingSelection() seems wrong to when entering InsertParagraphSeparatorCommand::doApply() : the anchor is the empty inner HTML but the associated OffsetInAnchor is 2. executing backward, this setup happened when the surroundContents function is called. It removes children of the inner HTML element and informs the doc about node removals, but for some reason the selection is not updated properly. I attached a patch to ASSERT that the m_offset is valid, but this is causing failures. ASSERTION FAILED: startBlock->firstChild() ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp(275) : virtual void WebCore::InsertParagraphSeparatorCommand::doApply() (rr) b InsertParagraphSeparatorCommand::doApply() (rr) rc (rr) (rr) delete (rr) p showTree(endingSelection().m_start) #document 0x61f00001dc80 (renderer 0x6160003ce480) html 0x60c0002a6940 (renderer (nil)) (needs style recalc) HTML 0x60c0002a6a00 (renderer 0x61200007d740) * HTML 0x60c0002ab500 (renderer 0x61200006c1c0) #text 0x60b0000e92a0 "0" DIV 0x60c0002aac00 (renderer 0x61200006c340) HTML 0x60c0002aaa80 (renderer 0x61200006c4c0) HTML 0x60c0002b1c80 (renderer 0x6120000a1140) offset, offset:2 $1 = void (rr) watch -l endingSelection().m_start Hardware watchpoint 3: -location endingSelection().m_start (rr) rc (rr) delete (rr) reverse-f (rr) (rr) bt #0 0x00007f65de74ea03 in WebCore::EditCommand::EditCommand(WebCore::Document&, WebCore::EditAction) (this=0x6120000a1440, document=..., editingAction=WebCore::EditAction::TypingInsertParagraph) at ../../Source/WebCore/editing/EditCommand.cpp:128 #1 0x00007f65e29d508c in WebCore::CompositeEditCommand::CompositeEditCommand(WebCore::Document&, WebCore::EditAction) (this=0x6120000a1440, document=..., editingAction=WebCore::EditAction::TypingInsertParagraph) at ../../Source/WebCore/editing/CompositeEditCommand.cpp:335 #2 0x00007f65de820feb in WebCore::InsertParagraphSeparatorCommand::InsertParagraphSeparatorCommand(WebCore::Document&, bool, bool, WebCore::EditAction) (this=0x6120000a1440, document=..., mustUseDefaultParagraphElement=false, pasteBlockqutoeIntoUnquotedArea=false, editingAction=WebCore::EditAction::TypingInsertParagraph) at ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:65 #3 0x00007f65de7737c0 in WebCore::InsertParagraphSeparatorCommand::create(WebCore::Document&, bool, bool, WebCore::EditAction) (document=..., useDefaultParagraphElement=false, pasteBlockqutoeIntoUnquotedArea=false, editingAction=WebCore::EditAction::TypingInsertParagraph) at ../../Source/WebCore/editing/InsertParagraphSeparatorCommand.h:38 (rr) p showTree(m_document->selection().selection()) #document 0x61f00001dc80 (renderer 0x6160003ce480) html 0x60c0002a6940 (renderer (nil)) (needs style recalc) HTML 0x60c0002a6a00 (renderer 0x61200007d740) SE HTML 0x60c0002ab500 (renderer 0x61200006c1c0) #text 0x60b0000e92a0 "0" DIV 0x60c0002aac00 (renderer 0x61200006c340) HTML 0x60c0002aaa80 (renderer 0x61200006c4c0) HTML 0x60c0002b1c80 (renderer 0x6120000a1140) start: offset, offset:2 end: offset, offset:2 $2 = void (rr) watch -l m_document->selection().selection().m_start (rr) rc (rr) delete (rr) bt #0 0x00007f65dd5bb208 in WebCore::Position::operator=(WebCore::Position const&) (this=0x613000082bf8) at ../../Source/WebCore/dom/Position.h:54 #1 0x00007f65de8ec09c in WebCore::VisibleSelection::setWithoutValidation(WebCore::Position const&, WebCore::Position const&) (this=0x613000082bb8, anchor=..., focus=...) at ../../Source/WebCore/editing/VisibleSelection.cpp:467 #2 0x00007f65de7d5985 in WebCore::FrameSelection::respondToNodeModification(WebCore::Node&, bool, bool, bool, bool) (this=0x613000082b80, node=warning: RTTI symbol not found for class 'WebCore::Text' ..., baseRemoved=true, extentRemoved=true, startRemoved=true, endRemoved=true) at ../../Source/WebCore/editing/FrameSelection.cpp:560 #3 0x00007f65de7d55cf in WebCore::FrameSelection::nodeWillBeRemoved(WebCore::Node&) (this=0x613000082b80, node=warning: RTTI symbol not found for class 'WebCore::Text' ...) at ../../Source/WebCore/editing/FrameSelection.cpp:541 #4 0x00007f65de27be09 in WebCore::Document::nodeChildrenWillBeRemoved(WebCore::ContainerNode&) (this=0x61f00001dc80, container=...) at ../../Source/WebCore/dom/Document.cpp:4816 #5 0x00007f65de1bc2bc in WebCore::ContainerNode::removeAllChildrenWithScriptAssertion(WebCore::ContainerNode::ChildChange::Source, WebCore::ContainerNode::DeferChildrenChanged) (this=0x60c0002ab500, source=WebCore::ContainerNode::ChildChange::Source::API, deferChildrenChanged=WebCore::ContainerNode::DeferChildrenChanged::No) at ../../Source/WebCore/dom/ContainerNode.cpp:120 #6 0x00007f65de1af59c in WebCore::ContainerNode::removeChildren() (this=0x60c0002ab500) at ../../Source/WebCore/dom/ContainerNode.cpp:715 #7 0x00007f65de1aef4f in WebCore::ContainerNode::replaceAll(WebCore::Node*) (this=0x60c0002ab500, node=0x0) at ../../Source/WebCore/dom/ContainerNode.cpp:670 #8 0x00007f65de5a6c0b in WebCore::Range::surroundContents(WebCore::Node&) (this=0x607000103100, newParent=...) at ../../Source/WebCore/dom/Range.cpp:827 (rr) reverse-f (rr) (rr) (rr) (rr) (rr) (rr) (rr) (rr) p showTree(newParent.document().selection().selection()) #document 0x61f00001dc80 (renderer 0x6160003ce480) html 0x60c0002a6940 (renderer (nil)) (needs style recalc) HTML 0x60c0002a6a00 (renderer 0x61200007d740) HTML 0x60c0002ab500 (renderer 0x61200006c1c0) svg 0x61300008a0c0 (renderer 0x615000258d00) svg 0x61300008a280 (renderer 0x615000258f80) SE #text 0x60b0000e7a90 "[" DIV 0x60c0002aac00 (renderer 0x61200006c340) HTML 0x60c0002aaa80 (renderer 0x61200006c4c0) start: legacy, offset, offset:0 end: legacy, offset, offset:0 $3 = void (rr) n (rr) p showTree(newParent.document().selection().selection()) #document 0x61f00001dc80 (renderer 0x6160003ce480) html 0x60c0002a6940 (renderer (nil)) (needs style recalc) HTML 0x60c0002a6a00 (renderer 0x61200007d740) SE HTML 0x60c0002ab500 (renderer 0x61200006c1c0) DIV 0x60c0002aac00 (renderer 0x61200006c340) HTML 0x60c0002aaa80 (renderer 0x61200006c4c0) start: offset, offset:2 end: offset, offset:2 $4 = void

Frédéric Wang (:fredw)

Comment 14 2021-06-02 06:31:53 PDT

Created attachment 430341 [details] Patch for ASSERTION FAILED: startBlock->firstChild()

Frédéric Wang (:fredw)

Comment 15 2021-06-02 06:33:45 PDT

(In reply to Frédéric Wang (:fredw) from comment #13) > It removes children > of the inner HTML element and informs the doc about node removals, but for > some reason the selection is not updated properly. The FrameSelection::respondToNodeModification does not seem to take into account upcoming removal of children when updating the selection, leaving it in a bad state. I uploaded a patch to work around that.

Frédéric Wang (:fredw)

Comment 16 2021-06-02 06:47:15 PDT

Created attachment 430342 [details] Patch for ASSERTION FAILED: refNode

Ryosuke Niwa

Comment 17 2021-06-07 15:11:55 PDT

Comment on attachment 430341 [details] Patch for ASSERTION FAILED: startBlock->firstChild() Looks like there are real test failures?

Frédéric Wang (:fredw)

Comment 18 2021-06-08 00:20:27 PDT

Comment on attachment 429872 [details] Add an assert for Position's offset Not going to take this.

Frédéric Wang (:fredw)

Comment 19 2021-06-08 00:20:47 PDT

Comment on attachment 430342 [details] Patch for ASSERTION FAILED: refNode This part is handled by bug 226527.

Ryosuke Niwa

Comment 20 2021-06-09 01:08:49 PDT

Looks like a bunch of tests are failing?

Frédéric Wang (:fredw)

Comment 21 2021-06-09 01:21:57 PDT

Comment on attachment 430341 [details] Patch for ASSERTION FAILED: startBlock->firstChild() Yes, sorry I wanted to check this again later. removing review for now.

Frédéric Wang (:fredw)

Comment 22 2021-06-09 05:47:46 PDT

Created attachment 430956 [details] Patch for ASSERTION FAILED: startBlock->firstChild()

Frédéric Wang (:fredw)

Comment 23 2021-06-15 02:41:47 PDT

Created attachment 431417 [details] Patch (fix offset) This fixes the offset, but that actually does not help the bad determination of startBlock/canonicalPos in InsertParagraphSeparatorCommand::doApply()

Frédéric Wang (:fredw)

Comment 24 2021-06-15 06:12:43 PDT

Created attachment 431430 [details] Tentative patch

Ryosuke Niwa

Comment 25 2021-06-15 14:59:27 PDT

Comment on attachment 431430 [details] Tentative patch View in context: https://bugs.webkit.org/attachment.cgi?id=431430&action=review > Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:192 > + if (!startBlock->contains(visiblePos.deepEquivalent().containerNode())) > + return; > + Why do we need to do this?

Frédéric Wang (:fredw)

Comment 26 2021-06-15 22:56:55 PDT

Comment on attachment 431430 [details] Tentative patch View in context: https://bugs.webkit.org/attachment.cgi?id=431430&action=review >> Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp:192 >> + > > Why do we need to do this? Yeah, so let me summarize what's happening since it's a bit complicate. First, below are the values when startBlock/canonicalPos are initialized in InsertParagraphSeparatorCommand::doApply: (rr) p showTree(insertionPosition) #document 0x61f00001dc80 (renderer 0x6160003cf080) html 0x60c0002a5d40 (renderer (nil)) (needs style recalc) HTML 0x60c0002a5e00 (renderer 0x61200007dbc0) * HTML 0x60c0002aa900 (renderer 0x61200006c640) #text 0x60b0000e8f30 "0" DIV 0x60c0002aa000 (renderer 0x61200006c7c0) HTML 0x60c0002a9e80 (renderer 0x61200006c940) HTML 0x60c0002b1080 (renderer 0x6120000916c0) offset, offset:2 $1 = void (rr) p showTree(startBlock) Cannot resolve function showTree to any overloaded instance (rr) p showTree((Node*) startBlock) #document 0x61f00001dc80 (renderer 0x6160003cf080) html 0x60c0002a5d40 (renderer (nil)) (needs style recalc) HTML 0x60c0002a5e00 (renderer 0x61200007dbc0) * HTML 0x60c0002aa900 (renderer 0x61200006c640) #text 0x60b0000e8f30 "0" DIV 0x60c0002aa000 (renderer 0x61200006c7c0) HTML 0x60c0002a9e80 (renderer 0x61200006c940) HTML 0x60c0002b1080 (renderer 0x6120000916c0) $2 = void (rr) p showTree(canonicalPos) #document 0x61f00001dc80 (renderer 0x6160003cf080) html 0x60c0002a5d40 (renderer (nil)) (needs style recalc) HTML 0x60c0002a5e00 (renderer 0x61200007dbc0) HTML 0x60c0002aa900 (renderer 0x61200006c640) * #text 0x60b0000e8f30 "0" DIV 0x60c0002aa000 (renderer 0x61200006c7c0) HTML 0x60c0002a9e80 (renderer 0x61200006c940) HTML 0x60c0002b1080 (renderer 0x6120000916c0) legacy, offset, offset:0 $3 = void Note that insertionPosition is in a bad state (empty <html> anchor node but offset equal to 2). This is explained and fixed by attachment 431417 [details] and I believe we can just rebase the expectation of the failing test if we want to take it, but that's really a separate issue here anyway... For this testcase, we have the same output for insertionPosition/visiblePos when visiblePos is initialized a few lines below. But the bad state of insertionPosition gets fixed by the upstream/downstream move. The interesting thing here is that startBlock is on the <html> element but the visible position is on the #text next sibling. The latter happens because Position::isCandidate() -- and a fortiori VisiblePosition::canonicalPosition() and VisiblePosition::VisiblePosition() -- skips <html> elements. This finally leads to the ASSERT failure, as explained in the preliminary debugging of comment 10. My understanding of the function (reading the apparent assumption of the comments e.g. the one before the ASSERT hit here) is that it is wrong to have the visible position outside the startBlock. So attachment 431430 [details] just performs an early return if that happens and that indeed fixes the issue without breaking any existing test on EWS. I also tried to fallback to applyCommandToComposite when the startBlock is <html> (since that's leading to special treatment for the calculation of the visible pos and booleans). However, that's causing another ASSERT in that case, IIRC that the position is not editable.

Frédéric Wang (:fredw)

Comment 27 2021-06-16 06:53:21 PDT

Created attachment 431541 [details] Patch Here is a real patch with test changelog & test. Reducing the test was a bit tricky because event_handler_538_DOMNodeRemoved was called several times. It still not perfect, but I was not able to reduce it more.

Frédéric Wang (:fredw)

Comment 28 2021-07-03 06:12:24 PDT

Created attachment 432849 [details] Patch (attempt to rebuild the dom tree and selection before crash) For the record, this is the follow-up work I tried to do on the test. I get the following assert messages: CONSOLE MESSAGE: bad anchorNode: [object Text] CONSOLE MESSAGE: bad anchorOffset: 0 CONSOLE MESSAGE: bad focusNode: [object Text] CONSOLE MESSAGE: bad focusOffset: 0 I can build the same DOM tree. As expected, I cannot set the invalid offset 2 but as previously explained that shouldn't matter here. However, I fail to set the anchor/focus on the inner <html> node, as that's the case with complex path with event handlers. Instead a text node is selected. So I'm not sure I can reduce this testcase further.

EWS

Comment 29 2021-07-26 13:47:38 PDT

Committed r280312 (239962@main): <https://commits.webkit.org/239962@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 431541 [details].

Note You need to log in before you can comment on or make changes to this bug.