1766956 - WARNING: nsRange::CompareBoundaryPoints(Range_Binding::START_TO_START) failed: file /builds/worker/checkouts/gecko/editor/libeditor/SelectionState.cpp:141

Status: RESOLVED FIXED

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

Description

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

This warning appears too much in mochitest log and WPT log.

Comment 1

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

It's called only by PlaceholderTransaction::StartSelectionEquals.
https://searchfox.org/mozilla-central/rev/86c98c486f03b598d0f80356b69163fd400ec8aa/editor/libeditor/PlaceholderTransaction.cpp#283

And it's also called only by PlaceholderTransaction::Merge.
https://searchfox.org/mozilla-central/rev/86c98c486f03b598d0f80356b69163fd400ec8aa/editor/libeditor/PlaceholderTransaction.cpp#248

It's called by:

1. PlaceholderTransaction::Merge to merge composition transactions
2. EditAggregateTransaction::Merge to merge child transactions

They are called TransactionManager::EndTransaction.
https://searchfox.org/mozilla-central/rev/86c98c486f03b598d0f80356b69163fd400ec8aa/editor/txmgr/TransactionManager.cpp#761

Comment 2

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

Looks like that one of the range is not positioned:
https://searchfox.org/mozilla-central/rev/ecd91b104714a8b2584a4c03175be50ccb3a7c67/dom/base/nsRange.cpp#1959
https://searchfox.org/mozilla-central/rev/ecd91b104714a8b2584a4c03175be50ccb3a7c67/editor/libeditor/SelectionState.cpp#107
https://searchfox.org/mozilla-central/rev/86c98c486f03b598d0f80356b69163fd400ec8aa/editor/libeditor/PlaceholderTransaction.cpp#282

Comment 3

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

Attachment: Bug 1766956 - part 1: Make `PlaceholderTransaction::StartSelectionEquals` call `SelectionState::Equals` only when both selection caches store caret positions r=m_kato!

The warnings are caused by comparing a range with non-positioned range, or
comparing non-positioned ranges. And SelectionState::IsCollapsed() may
return true when it has and only has a non-positioned range.

Therefore, this patch adds new method which checks whether the selection cache
points a caret position or not instead of the "collapsed" state, and make
PlaceholderTransaction::StartSelectionEquals check is result before calling
SelectionState::Equals.

Comment 4

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

Attachment: Bug 1766956 - part 2: Make `PlaceholderTransaction::Merge` return `false` if the caret points of merging transactions are in different root nodes r=m_kato!

This is the most cases of the warnings. If previous transactions occurred in
now disconnected subtree, e.g., they are replaced by setting innerHTML to
check different scenarios, nsRange::CompareBoundaryPoints return error.
Therefore, this case should be handled before comparing the range boundaries
and logged for making easier to debug.

Depends on D145225

Comment 5

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

Attachment: Bug 1766956 - part 3: Make `SelectionState` stop using `nsRange` to compare DOM points r=m_kato!

The creation and initialization cost of nsRange is expensive. And just
comparing DOM points does not require the checks of ancestors which are
required by nsRange to observe DOM mutations. Therefore, although the
logic is not exactly same, we can skip the additional unnecessary cost.
Note that the Editor module checks without nsRange's strict initialization
in most places so that it must not be required to do the strict check only in
SelectionState.

Additionally, this moves some definitions of SelectionState methods to
its header for making them inlined since the dependency is now reduced.

Finally, some methods are renamed to align to Selection API.

Depends on D145226

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f222a317c1a
part 1: Make `PlaceholderTransaction::StartSelectionEquals` call `SelectionState::Equals` only when both selection caches store caret positions r=m_kato
https://hg.mozilla.org/integration/autoland/rev/6f9a9774db23
part 2: Make `PlaceholderTransaction::Merge` return `false` if the caret points of merging transactions are in different root nodes r=m_kato

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/377f41f6d339
part 3: Make `SelectionState` stop using `nsRange` to compare DOM points r=m_kato

Comment 8

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

https://hg.mozilla.org/mozilla-central/rev/7f222a317c1a
https://hg.mozilla.org/mozilla-central/rev/6f9a9774db23
https://hg.mozilla.org/mozilla-central/rev/377f41f6d339