1734507 - Assertion failure: aEnd.Offset(RawRangeBoundary::OffsetFilter::kValidOffsets).value() == mEditorBase->GetRoot()->Length(), at editor/libeditor/DeleteRangeTransaction.cpp:227

Status: RESOLVED FIXED

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

Description

[Mats Palmgren (inactive)]

Fwiw, I got this fatal assertion when editing a CSS property value in DevTools (using a local debug build on Linux):

Assertion failure: aEnd.Offset(RawRangeBoundary::OffsetFilter::kValidOffsets).value() == mEditorBase->GetRoot()->Length(), at editor/libeditor/DeleteRangeTransaction.cpp:227

Sorry, I don't have exact STR, nor did I try to reproduce it...

Comment 1

[Mirko Brodesser (:mbrodesser-Igalia), offline until 10th of Feb]

I guess that violation isn't very dangerous, because textNode->TextDataLength() is called afterwards at https://searchfox.org/mozilla-central/rev/477950cf9ca9c9bb5ff6f34e0d0f6ca4718ea798/editor/libeditor/DeleteRangeTransaction.cpp#233-235, but I'm not very familiar with that code.

Kagami: since the violated assertion, https://searchfox.org/mozilla-central/rev/477950cf9ca9c9bb5ff6f34e0d0f6ca4718ea798/editor/libeditor/DeleteRangeTransaction.cpp#225-227, was added by you, could you please set severity and priority for this ticket?

Incidental remark: having MOZ_ASSERT_EQ would be nice.

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

Hmm, something does look odd there, but if that doesn't cause a crash on release build I guess it's fine to give S3.

Some work need to be done to make "100% sure nothing does so", so filed bug 1734846.

Comment 3

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

Could be caused by the selection selects only the text node, but there is a trailing <br>, e.g., in <textarea>? So, perhaps, the MOZ_ASSERT should check whether it's less than the child count of the root (anonymous <div>)?

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

That would be also weird since I think we removed trailing <br> from TextEditors 👀

Comment 5

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

Okay, I got it. If our internal code select from and to outside the text node, i.e., the container is the anonymous <div> element, TextEditor tries to delete the text node due to its completely in the range.

Comment 6

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

Attachment: Bug 1734507 - Make `TextEditor` guarantee that ranges to delete are in the text node r=saschanaz!

Our internal code can touch internal Selection object of <input> and
<textarea>, thus, selection ranges in them may set selection ranges not
in the text node. Therefore, we should make TextEditor instance won't
try to delete ranges which crosses the range boundaries of the text node.

In a followup bug (perhaps, bug 1734846), we should split the deletion code into
TextEditor and HTMLEditor for making the code simpler and never regressed.

Comment 7

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

Attachment: Bug 1734507 - Make `TextEditor` guarantee that ranges to delete are in the text node r=saschanaz!

Our internal code can touch internal Selection object of <input> and
<textarea>, thus, selection ranges in them may be not in the text node.
Therefore, we should make TextEditor instance try to shrink the deleting
ranges into the text node first.

In a followup bug (perhaps, bug 1734846), we should split the deletion code into
TextEditor and HTMLEditor for making the code simpler and never regressed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a967b7bb1775
Make `TextEditor` guarantee that ranges to delete are in the text node r=saschanaz

Comment 9

[Norisz Fay [:noriszfay]]

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