1793873 - Assertion failure: aContentToKeep.GetParentNode() == aContentToRemove.GetParentNode(), at /builds/worker/checkouts/gecko/editor/libeditor/HTMLEditor.cpp:5264

Status: VERIFIED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20221004-8454bb0c09fe (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

Assertion failure: aContentToKeep.GetParentNode() == aContentToRemove.GetParentNode(), at /builds/worker/checkouts/gecko/editor/libeditor/HTMLEditor.cpp:5264

#0 0x7f683df1fe80 in mozilla::HTMLEditor::DoJoinNodes(nsIContent&, nsIContent&, mozilla::JoinNodesDirection) /gecko/editor/libeditor/HTMLEditor.cpp:5263:3
#1 0x7f683dff4fc5 in mozilla::SplitNodeTransaction::UndoTransaction() /gecko/editor/libeditor/SplitNodeTransaction.cpp:194:22
#2 0x7f683de10622 in mozilla::EditAggregateTransaction::UndoTransaction() /gecko/editor/libeditor/EditAggregateTransaction.cpp:65:52
#3 0x7f683dfea691 in mozilla::PlaceholderTransaction::UndoTransaction() /gecko/editor/libeditor/PlaceholderTransaction.cpp:75:43
#4 0x7f683e084893 in mozilla::TransactionItem::UndoTransaction(mozilla::TransactionManager*) /gecko/editor/txmgr/TransactionItem.cpp:105:21
#5 0x7f683e086d4d in mozilla::TransactionManager::Undo() /gecko/editor/txmgr/TransactionManager.cpp:111:34
#6 0x7f683de20541 in mozilla::EditorBase::UndoAsAction(unsigned int, nsIPrincipal*) /gecko/editor/libeditor/EditorBase.cpp:1034:11
#7 0x7f68383b3175 in mozilla::dom::Document::ExecCommand(nsTSubstring<char16_t> const&, bool, nsTSubstring<char16_t> const&, nsIPrincipal&, mozilla::ErrorResult&) /gecko/dom/base/Document.cpp:5429:37
#8 0x7f6839f662e4 in mozilla::dom::Document_Binding::execCommand(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/DocumentBinding.cpp:4149:36
#9 0x7f683a43be8f in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) /gecko/dom/bindings/BindingUtils.cpp:3287:13
#10 0x7f6844ca3653 in CallJSNative /gecko/js/src/vm/Interpreter.cpp:459:13
#11 0x7f6844ca3653 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /gecko/js/src/vm/Interpreter.cpp:547:12
#12 0x7f6844c91fd9 in InternalCall /gecko/js/src/vm/Interpreter.cpp:614:10
#13 0x7f6844c91fd9 in CallFromStack /gecko/js/src/vm/Interpreter.cpp:619:10
#14 0x7f6844c91fd9 in Interpret(JSContext*, js::RunState&) /gecko/js/src/vm/Interpreter.cpp:3375:16
#15 0x7f6844c775de in js::RunScript(JSContext*, js::RunState&) /gecko/js/src/vm/Interpreter.cpp:431:13
#16 0x7f6844ca3775 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /gecko/js/src/vm/Interpreter.cpp:579:13
#17 0x7f6844ca521e in InternalCall /gecko/js/src/vm/Interpreter.cpp:614:10
#18 0x7f6844ca521e in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /gecko/js/src/vm/Interpreter.cpp:646:8
#19 0x7f68432874e5 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /gecko/js/src/vm/CallAndConstruct.cpp:117:10
#20 0x7f683a044fdf in mozilla::dom::EventHandlerNonNull::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/EventHandlerBinding.cpp:65:37
#21 0x7f683af0cc03 in void mozilla::dom::EventHandlerNonNull::Call<nsCOMPtr<mozilla::dom::EventTarget> >(nsCOMPtr<mozilla::dom::EventTarget> const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventHandlerBinding.h:82:12
#22 0x7f683af0b178 in mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) /gecko/dom/events/JSEventHandler.cpp:201:12
#23 0x7f683aed1268 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) /gecko/dom/events/EventListenerManager.cpp:1316:22
#24 0x7f683aed278b in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) /gecko/dom/events/EventListenerManager.cpp:1506:17
#25 0x7f683aec090e in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /gecko/dom/events/EventDispatcher.cpp:348:17
#26 0x7f683aebf171 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /gecko/dom/events/EventDispatcher.cpp:550:16
#27 0x7f683aec3355 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /gecko/dom/events/EventDispatcher.cpp:1119:11
#28 0x7f683aec8cd1 in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*) /gecko/dom/events/EventDispatcher.cpp
#29 0x7f6838781434 in nsINode::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&) /gecko/dom/base/nsINode.cpp:1373:17
#30 0x7f683aedfb73 in mozilla::dom::EventTarget::DispatchEvent(mozilla::dom::Event&) /gecko/dom/events/EventTarget.cpp:180:13
#31 0x7f683ae47960 in mozilla::AsyncEventDispatcher::Run() /gecko/dom/events/AsyncEventDispatcher.cpp:69:12
#32 0x7f683515db92 in mozilla::RunnableTask::Run() /gecko/xpcom/threads/TaskController.cpp:538:16
#33 0x7f683511e3dd in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:851:26
#34 0x7f683511b548 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /gecko/xpcom/threads/TaskController.cpp:683:15
#35 0x7f683511bc70 in mozilla::TaskController::ProcessPendingMTTask(bool) /gecko/xpcom/threads/TaskController.cpp:461:36
#36 0x7f6835166ac1 in operator() /gecko/xpcom/threads/TaskController.cpp:187:37
#37 0x7f6835166ac1 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
#38 0x7f683513f717 in nsThread::ProcessNextEvent(bool, bool*) /gecko/xpcom/threads/nsThread.cpp:1205:16
#39 0x7f6835149b94 in NS_ProcessNextEvent(nsIThread*, bool) /gecko/xpcom/threads/nsThreadUtils.cpp:465:10
#40 0x7f68368ec05f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /gecko/ipc/glue/MessagePump.cpp:85:21
#41 0x7f6836769b61 in RunInternal /gecko/ipc/chromium/src/base/message_loop.cc:381:10
#42 0x7f6836769b61 in RunHandler /gecko/ipc/chromium/src/base/message_loop.cc:374:3
#43 0x7f6836769b61 in MessageLoop::Run() /gecko/ipc/chromium/src/base/message_loop.cc:356:3
#44 0x7f683dc00447 in nsBaseAppShell::Run() /gecko/widget/nsBaseAppShell.cpp:150:27
#45 0x7f6842b77f87 in nsAppStartup::Run() /gecko/toolkit/components/startup/nsAppStartup.cpp:295:30
#46 0x7f6842da5f45 in XREMain::XRE_mainRun() /gecko/toolkit/xre/nsAppRunner.cpp:5723:22
#47 0x7f6842da7c9e in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /gecko/toolkit/xre/nsAppRunner.cpp:5916:8
#48 0x7f6842da8a1b in XRE_main(int, char**, mozilla::BootstrapConfig const&) /gecko/toolkit/xre/nsAppRunner.cpp:5972:21
#49 0x55af5b1dc806 in do_main(int, char**, char**) /gecko/browser/app/nsBrowserApp.cpp:226:22
#50 0x55af5b1dbaa7 in main /gecko/browser/app/nsBrowserApp.cpp:430:16
#51 0x7f685d520082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/../csu/libc-start.c:308:16
#52 0x55af5b11b9b9 in _start (/home/worker/builds/m-c-20221004094418-fuzzing-asan-opt/firefox+0x7a9b9) (BuildId: c4297499640c9d6558d0d1dd79b536c20cfcea4f)

Comment 1

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20221005215113-517d690052a2.
The bug appears to have been introduced in the following build range:

Start: ab088885dcd0be78848c4f2cd7aa791d334332a0 (20220930192658)
End: 63fbc22d22232060e734e218ec6feec0bacc645a (20221001003034)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ab088885dcd0be78848c4f2cd7aa791d334332a0&tochange=63fbc22d22232060e734e218ec6feec0bacc645a

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1792387

:masayuki, since you are the author of the regressor, bug 1792387, could you take a look? Also, could you set the severity field?

For more information, please visit auto_nag documentation.

Comment 3

[Mayank Bansal]

Got a crash : https://crash-stats.mozilla.org/report/index/cde66396-1bb2-455a-9416-493a70221006#tab-bugzilla

Comment 4

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

I'm still not sure the reason why hitting the assertion in the testcase, but I'll take a look soon.

Comment 5

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

Comment on attachment 9297364 [details]
testcase.html

Ah, it seems that error event is fired at least twice, and surprisingly we split the <textarea> at insertParagraph, and it caused unexpected DOM tree. I'm still not sure whether it occurs in the more realistic cases

Comment 6

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

Ah, and anyway, it can run with undo command so that the re-joining nodes can be everywhere. So the assertion is wrong and DoJoinNodes should not work with assuming that they are in same parent node.

Comment 7

[Donal Meehan [:dmeehan]]

:masayuki could you review the severity based on comment 5 and comment 6? Is this an S2

Comment 8

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

Yeah, I think so. This causes a crash in beta and nightly.

Comment 9

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

The patches for this bug come tomorrow, so must be fixed in current cycle.

Comment 10

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

Attachment: Bug 1793873 - part 1: Make some elements non-splittable r=m_kato!

One of the reason why the reported test case causes odd DOM tree result is,
HTMLEditUtils::IsSplittableNode returns false for <textarea>. Then,
insertParagraph command with Selection collapsed in <textarea> causes
splitting the <textarea> and it's not split at same parent.

Comment 11

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

Attachment: Bug 1793873 - Make `HTMLEditor::DoSplitNode` stop assuming that joining nodes are in same parent r=m_kato!

Between splitting a node and undoing it, web apps can move split nodes anywhere.
Therefore, it shouldn't assume they are always in same parent node, and
RangeUpdater::SelAdjJoinNodes needs to handle it correctly.

Unfortunately, RangeUpdater::SelAdjJoinNodes cannot handle nested cases
correctly, e.g., right node was in aRemovedContent or right node was in
the container of aStartOfRightContent.GetContainer(). However, it's not
a new regression, and such complicated situation breaks undoing anyway.
Therefore, I think that we don't need to care about it for now.

Depends on D159229

Comment 12

[Phabricator Automation]

Comment on attachment 9298325 [details]
Bug 1793873 - part 1: Make some elements non-splittable r=m_kato!

Revision D159229 was moved to bug 1795179. Setting attachment 9298325 [details] to obsolete.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26035e818273
Make `HTMLEditor::DoSplitNode` stop assuming that joining nodes are in same parent r=m_kato

Comment 14

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36450 for changes under testing/web-platform/tests

Comment 15

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/26035e818273

Comment 16

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 17

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20221014095137-9142cc0a7a33.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.