1429523 - Assertion failure: mPreCharacterDataChangeLength >= aInfo->mChangeEnd - aInfo->mChangeStart (The computed length must be same as or larger than XP length), at /builds/worker/workspace/build/src/dom/events/IMEContentObserver.cpp:964

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase

Testcase found while fuzzing mozilla-central rev d5f42a23909e.

OS|Linux|0.0.0 Linux 4.4.0-104-generic #127-Ubuntu SMP Mon Dec 11 12:16:42 UTC 2017 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV|0x0|0
0|0|libxul.so|mozilla::IMEContentObserver::CharacterDataWillChange|hg:hg.mozilla.org/mozilla-central:dom/events/IMEContentObserver.cpp:d5f42a23909e|962|0x18
0|1|libxul.so|nsNodeUtils::CharacterDataWillChange|hg:hg.mozilla.org/mozilla-central:dom/base/nsNodeUtils.cpp:d5f42a23909e|137|0x17
0|2|libxul.so|nsGenericDOMDataNode::SetTextInternal|hg:hg.mozilla.org/mozilla-central:dom/base/nsGenericDOMDataNode.cpp:d5f42a23909e|309|0x5
0|3|libxul.so|nsGenericDOMDataNode::SetData|hg:hg.mozilla.org/mozilla-central:dom/base/nsGenericDOMDataNode.cpp:d5f42a23909e|184|0xa
0|4|libxul.so|nsGenericDOMDataNode::SetData|hg:hg.mozilla.org/mozilla-central:dom/base/nsGenericDOMDataNode.h:d5f42a23909e|185|0x5
0|5|libxul.so|mozilla::dom::CharacterDataBinding::set_data|s3:gecko-generated-sources:881ae6b0ba6cb87846d340b245161db1c27debf81ac26be321ac710fe3653d7986073ce8cb89daafa322faffdec0537850a364deac99573b8698fa26046e6c40/dom/bindings/CharacterDataBinding.cpp:|48|0x12
0|6|libxul.so|mozilla::dom::GenericBindingSetter|hg:hg.mozilla.org/mozilla-central:dom/bindings/BindingUtils.cpp:d5f42a23909e|3003|0x10
0|7|libxul.so|js::CallJSNative|hg:hg.mozilla.org/mozilla-central:js/src/jscntxtinlines.h:d5f42a23909e|291|0x6
0|8|libxul.so|js::InternalCallOrConstruct|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d5f42a23909e|473|0xf
0|9|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d5f42a23909e|522|0xd
0|10|libxul.so|js::Call|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d5f42a23909e|541|0x5
0|11|libxul.so|js::CallSetter|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d5f42a23909e|670|0x5
0|12|libxul.so|SetExistingProperty|hg:hg.mozilla.org/mozilla-central:js/src/vm/NativeObject.cpp:d5f42a23909e|2736|0xb
0|13|libxul.so|js::NativeSetProperty<(js::QualifiedBool)1u>|hg:hg.mozilla.org/mozilla-central:js/src/vm/NativeObject.cpp:d5f42a23909e|2764|0x2f
0|14|libxul.so|js::SetProperty|hg:hg.mozilla.org/mozilla-central:js/src/vm/NativeObject.h:d5f42a23909e|1637|0x34
0|15|libxul.so|Interpret|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d5f42a23909e|270|0x5
0|16|libxul.so|js::RunScript|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d5f42a23909e|423|0xb
0|17|libxul.so|js::ExecuteKernel|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d5f42a23909e|706|0x5
0|18|libxul.so|js::Execute|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:d5f42a23909e|739|0x5
0|19|libxul.so|ExecuteScript|hg:hg.mozilla.org/mozilla-central:js/src/jsapi.cpp:d5f42a23909e|4693|0x11
0|20|libxul.so|ExecuteScript|hg:hg.mozilla.org/mozilla-central:js/src/jsapi.cpp:d5f42a23909e|4712|0x8
0|21|libxul.so|nsJSUtils::ExecutionContext::CompileAndExec|hg:hg.mozilla.org/mozilla-central:dom/base/nsJSUtils.cpp:d5f42a23909e|266|0xc
0|22|libxul.so|mozilla::dom::ScriptLoader::EvaluateScript|hg:hg.mozilla.org/mozilla-central:dom/script/ScriptLoader.cpp:d5f42a23909e|2233|0x12
0|23|libxul.so|mozilla::dom::ScriptLoader::ProcessRequest|hg:hg.mozilla.org/mozilla-central:dom/script/ScriptLoader.cpp:d5f42a23909e|1876|0xb
0|24|libxul.so|mozilla::dom::ScriptLoader::ProcessScriptElement|hg:hg.mozilla.org/mozilla-central:dom/script/ScriptLoader.cpp:d5f42a23909e|1574|0xf
0|25|libxul.so|mozilla::dom::ScriptElement::MaybeProcessScript|hg:hg.mozilla.org/mozilla-central:dom/script/ScriptElement.cpp:d5f42a23909e|147|0x13
0|26|libxul.so|nsIScriptElement::AttemptToExecute|hg:hg.mozilla.org/mozilla-central:dom/script/nsIScriptElement.h:d5f42a23909e|240|0x3
0|27|libxul.so|nsHtml5TreeOpExecutor::RunScript|hg:hg.mozilla.org/mozilla-central:parser/html/nsHtml5TreeOpExecutor.cpp:d5f42a23909e|736|0x10
0|28|libxul.so|nsHtml5TreeOpExecutor::RunFlushLoop|hg:hg.mozilla.org/mozilla-central:parser/html/nsHtml5TreeOpExecutor.cpp:d5f42a23909e|540|0x8
0|29|libxul.so|nsHtml5ExecutorFlusher::Run|hg:hg.mozilla.org/mozilla-central:parser/html/nsHtml5StreamParser.cpp:d5f42a23909e|131|0x10
0|30|libxul.so|mozilla::SchedulerGroup::Runnable::Run|hg:hg.mozilla.org/mozilla-central:xpcom/threads/SchedulerGroup.cpp:d5f42a23909e|395|0x1c
0|31|libxul.so|nsThread::ProcessNextEvent|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:d5f42a23909e|1040|0x15
0|32|libxul.so|NS_ProcessNextEvent|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:d5f42a23909e|517|0x11
0|33|libxul.so|mozilla::ipc::MessagePump::Run|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:d5f42a23909e|97|0xa
0|34|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:d5f42a23909e|326|0x17
0|35|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:d5f42a23909e|319|0x8
0|36|libxul.so|nsBaseAppShell::Run|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:d5f42a23909e|157|0xd
0|37|libxul.so|XRE_RunAppShell|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:d5f42a23909e|877|0x11
0|38|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:d5f42a23909e|269|0x5
0|39|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:d5f42a23909e|326|0x17
0|40|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:d5f42a23909e|319|0x8
0|41|libxul.so|XRE_InitChildProcess|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:d5f42a23909e|703|0x8
0|42|firefox|content_process_main|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:d5f42a23909e|63|0x14
0|43|firefox|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:d5f42a23909e|280|0x11
0|44|libc-2.23.so||||0x20830
0|45|firefox|MOZ_ReportAssertionFailure|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:d5f42a23909e|165|0x5

Comment 1

[Andreas Farre [:farre]]

I can't reproduce that crash, is it supposed to happen every time?

Comment 2

[Andreas Farre [:farre]]

Sorry, missed that it was an assert, sorry.

Comment 3

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

IMEContentObserver (not ContentEventHandler) doesn't assume that it treats data nodes which are not text nodes. So, we should make ignore such nodes in both classes.

Comment 4

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

s/(not ContentEventHandler)/(and ContentEventHandler)

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

ah yes, only Text and CDATA should be handled

Comment 6

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

Ah, CDATA. Looks like it's a derived class of Text:
https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/dom/xml/CDATASection.h#17

So, we should handle only when nsINode::AsText() returns non-nullptr.

Comment 7

[Jason Kratzer [:jkratzer]]

Attachment: Additional testcase targeting comments

Comment 8

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210223085042-916497e295fe.

Comment 9

[Jens Stutte [:jstutte]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Still not recoverd perfectly) from comment #6)

Ah, CDATA. Looks like it's a derived class of Text:
https://searchfox.org/mozilla-central/rev/
7fb999d1d39418fd331284fab909df076b967ac6/dom/xml/CDATASection.h#17

So, we should handle only when nsINode::AsText() returns non-nullptr.

Masayuki, reads like an easy thing to do?

Comment 10

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

The testcase is now not reproducible because of perhaps, focus handling timing was changed.

Comment 11

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

Attachment: testcase

Comment 12

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

Attachment: Bug 1429523 - Make IMEContentObserver ignore character data change notifications of invisible data nodes r=smaug!

E.g., comment node is an invisible data node, and it's not handled by
HTMLEditor nor ContentEventHandler. Therefore, IMEContentObserver
should ignore invisible data nodes, which are not derived from dom::Text.

Depends on D107589

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45e3e32a1c13
Make IMEContentObserver ignore character data change notifications of invisible data nodes r=smaug

Comment 14

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/45e3e32a1c13

Comment 15

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

:masayuki, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 16

[Marco Castelluccio [:marco]]

Sorry, bug in the bot.