822734 - Assertion failure: mNextStartRef == aFirstNewContent || !mStartOffsetWasIncremented && !mEndOffsetWasIncremented && mAssertNextInsertOrAppendIndex == -1

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Assertion failure: !mStartOffsetWasIncremented && !mEndOffsetWasIncremented && mAssertNextInsertOrAppendIndex == -1 (splitText failed to notify insert/append?), at content/base/src/nsRange.cpp:618

Not sure if editor bug, range bug, or splitText bug.

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Jason Kratzer [:jkratzer]]

Attachment: Additional testcase

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Assertion failure: !mNextStartRef, at M:/src/dom/base/nsRange.cpp:406
#01: nsRange::CharacterDataChanged (M:\src\dom\base\nsRange.cpp:406)
#02: mozilla::dom::MutationObservers::NotifyCharacterDataChanged (M:\src\dom\base\MutationObservers.cpp:128)
#03: mozilla::dom::CharacterData::SetTextInternal (M:\src\dom\base\CharacterData.cpp:336)
#04: mozilla::dom::CharacterData::SetData (M:\src\dom\base\CharacterData.cpp:157)
#05: mozilla::EditorBase::DoSetText (M:\src\editor\libeditor\EditorBase.cpp:2544)
#06: mozilla::EditorBase::SetTextNodeWithoutTransaction (M:\src\editor\libeditor\EditorBase.cpp:3154)
#07: mozilla::TextEditor::SetTextWithoutTransaction (M:\src\editor\libeditor\TextEditSubActionHandler.cpp:579)
#08: mozilla::TextEditor::SetTextAsSubAction (M:\src\editor\libeditor\TextEditor.cpp:425)
#09: mozilla::TextEditor::SetTextAsAction (M:\src\editor\libeditor\TextEditor.cpp:397)
#10: mozilla::TextControlState::SetValueWithTextEditor (M:\src\dom\html\TextControlState.cpp:2815)
#11: mozilla::TextControlState::SetValue (M:\src\dom\html\TextControlState.cpp:2697)
#12: mozilla::dom::HTMLTextAreaElement::Reset (M:\src\dom\html\HTMLTextAreaElement.cpp:671)
#13: mozilla::dom::HTMLTextAreaElement::ContentChanged (M:\src\dom\html\HTMLTextAreaElement.cpp:854)
#14: mozilla::dom::MutationObservers::NotifyContentInserted (M:\src\dom\base\MutationObservers.cpp:185)
#15: nsINode::InsertChildBefore (M:\src\dom\base\nsINode.cpp:1613)
#16: mozilla::dom::Text::SplitText (M:\src\dom\base\Text.cpp:55)
#17: mozilla::dom::Text_Binding::splitText (M:\fx64-dbg\dom\bindings\TextBinding.cpp:67)
#18: mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions> (M:\src\dom\bindings\BindingUtils.cpp:3310)

The assertion is a state check.

It's set during a call of character data change.

• https://searchfox.org/mozilla-central/rev/f1dc2743777711c821d43f9911ee7c4447d60c8e/dom/base/nsRange.cpp#414
• https://searchfox.org/mozilla-central/rev/f1dc2743777711c821d43f9911ee7c4447d60c8e/dom/base/nsRange.cpp#334

Then, cleared when content appended or content inserted.

Therefore, I don't understand the correctness of the assertion. I wonder, for avoiding overwriting mNextStartRef, it could intended to:

  if (aInfo.mDetails &&
      aInfo.mDetails->mType == CharacterDataChangeInfo::Details::eSplit) {
    MOZ_ASSERT(!mNextStartRef);
    AdjustNextRefsOnCharacterDataSplit(*aContent, aInfo);
  }

It's a replacement of nsRange::mAssertNextInsertOrAppendIndex which was removed in bug 1380367, and it was added in bug 803924.

Smaug: You're the reviewer of the original patch, do you have any ideas?

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Attachment: testcase (easier to read)

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Ah, I think that HTMLTextAreaElement should reset its subtree with a script runner.

Comment 6

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

Ah, I think the patch I commented was for this bug, not the bug it was attached. But now I lost the link to that patch.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Attachment: Bug 822734 - Make `HTMLTextAreaElement` handle the mutation changes after all ranges handle them r=smaug!

A mutation caused by a call of Text::SplitText is handled by 2 method calls,
CharacterDataChanged and ContentInserted, in nsRange. Therefore,
nsRange stores some nodes for the later one, but
HTMLTextAreaElement::ContentInserted is called before it and that causes
another mutation which causes calling nsRange::CharacterDataChanged again.
Therefore, the assertion detects the recursive call.

For avoiding this issue, HTMLTextAreaElement needs to wait that all ranges
handle the mutation first. Fortunately, ContentInserted is called with a
script blocker (*1). Therefore, HTMLTextAreaElement can use script runner
to reset the anonymous subtree.

1. https://searchfox.org/mozilla-central/rev/f1dc2743777711c821d43f9911ee7c4447d60c8e/dom/base/nsINode.cpp#1566,1610

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e001f4e845a4
Make `HTMLTextAreaElement` handle the mutation changes after all ranges handle them r=smaug

Comment 9

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

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

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/e001f4e845a4

Comment 11

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

Upstream PR merged by moz-wptsync-bot