1409520 - Assertion failure: node->GetChildAt(offset) == *aInOutChildAtOffset, at /builds/worker/workspace/build/src/editor/libeditor/EditorBase.cpp:2513

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Jason Kratzer [:jkratzer]]

Attachment: trigger.html

Testcase found while fuzzing mozilla-central rev 7b75416fb54c.

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: log_minidump.txt

Comment 2

[Ryan VanderMeulen [:RyanVM]]

INFO: Last good revision: f2b1253de1e684d57290ba086fc57e300870a91b
INFO: First bad revision: fdb1abbe808a069d2b6ca4ae7ed98969a3fb7314
INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f2b1253de1e684d57290ba086fc57e300870a91b&tochange=fdb1abbe808a069d2b6ca4ae7ed98969a3fb7314

Masayuki, can you please take a look while Ehsan is away?

Comment 3

[Jorg K (CEST = GMT+2)]

Fixed by bug 1407966?

Comment 4

[Ryan VanderMeulen [:RyanVM]]

On autoland rev e802283bbf21d14d431901dd286681cd1933bb97, it's still asserting, albeit a different now.

Assertion failure: !node->IsContainerNode() || node->Length() == static_cast<uint32_t>(offset) || node->GetChildAt(offset) == *aInOutChildAtOffset (|child| must be a child node at |offset| in |node| unless it's a text or some other data node, or after the last child), at z:/build/build/src/editor/libeditor/EditorBase.cpp:2517

Comment 5

[Jorg K (CEST = GMT+2)]

Hmm, that's the rephrased assertion Masayuki just landed in bug 1407966:
https://hg.mozilla.org/mozilla-central/rev/760469149d9d#l1.13

So there's still a gremlin in the box. Let's hope that bug 1408227 fixes it then.

(Unrelated: Bug 1407966 fixed an assert in the TB Mozmill test suite.)

Comment 6

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

Okay, I got what's the cause of this. That is, AutoTrackDOMPoint in WSRunObject::InsertText() may update aInOutParent and aInOutOffset but it doesn't modify aInOutChildAtOffset.

Comment 7

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b8ba68918317999981431680be6f7fc1d17cee6

Comment 8

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

Attachment: Bug 1409520 - part 0: Add automated test

The bug is a report for hitting MOZ_ASSERT, but we should also check the result
of the call of execCommand.

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

Comment 9

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

Attachment: Bug 1409520 - part 1: WSRunObject::InsertText() should update aInOutChildAtOffset after it might be modified

WSRunObject::InsertText() may delete given child node at offset with calling
DeleteChars() or CheckLeadingNBSP() before calling HTMLEditor::InsertTextImpl().

Therefore, even though using nsINode::GetChildAt() is slow, it needs to update
aInOutChildAtOffset after calling them.

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

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 8921154 [details]
Bug 1409520 - part 0: Add automated test

https://reviewboard.mozilla.org/r/192140/#review197532

Comment 11

[Makoto Kato [:m_kato]]

Comment on attachment 8921155 [details]
Bug 1409520 - part 1: WSRunObject::InsertText() should update aInOutChildAtOffset after it might be modified

https://reviewboard.mozilla.org/r/192142/#review197540

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e04990f5e538
part 0: Add automated test r=m_kato
https://hg.mozilla.org/integration/autoland/rev/ef85fd3a802c
part 1: WSRunObject::InsertText() should update aInOutChildAtOffset after it might be modified r=m_kato

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/e04990f5e538
https://hg.mozilla.org/mozilla-central/rev/ef85fd3a802c