1885822 - Make `WSRunScanner::ReachedCurrentBlockBoundary` never lie

Status: RESOLVED FIXED

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

Description

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

When it's true, it may not reach current block boundary. This is really annoying when you want to check whether it reaches current block boundary.

Comment 1

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

Attachment: Bug 1885822 - part 1: Make `WSScanResult` created with `WSType::InUncomposedDoc` if the scan is tried in uncomposed tree r=m_kato!

This case should never happen because nobody can check proper editable state
in uncomposed tree unless the uncomposed tree has its own editing host.
Therefore, this patch adds assertions into the scanning methods too.

Comment 2

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

Attachment: Bug 1885822 - part 2: Make `WSScanResult` always have non-null `mContent` if it reached something r=m_kato!

Currently, it checks whether mContent is nullptr or not even if the scanner
reached something. However, this makes the users of this object check whether
it has reasonable content or not and that makes the users messy. Therefore,
the scanning method should guarantee that it's always error if it does not reach
any content.

Depends on D207681

Comment 3

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

Attachment: Bug 1885822 - part 3: Make `WSScanResult` always have editable element with `mContent` if it reached current block boundary r=m_kato!

Different from the comment around setter of WSType::CurrentBlockBoundary,
neither HTMLEditUtils::GetNextLeafContentOrNextBlockElement nor
HTMLEditUtils::GetPreviousLeafContentOrPreviousBlockElement returns
nullptr when it reaches non-editable element, and the setters always set
mContent to aEditableBlockParentOrTopmostEditableInlineElement. Therefore,
mContent is always an editable element when ReachedCurrentBlockBoundary()
returns true.

Depends on D207682

Comment 4

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

Attachment: Bug 1885822 - part 4: Add `WSType::InlineEditingHostBoundary` r=m_kato!

When WSScanResult::ReachedCurrentBlockBoundary() returns true, it may have
reached inline editing host rather than editable block. Therefore, the
method name is definitely error-prone. This patch adds new state and all
users of WSType::CurrentBlockBoundary keeps checking the new type too for
keeping current behavior.

I think that we should make WSRunScanner aware of inline editing host next
to block boundary before removing odd check of
ReachedInlineEditingHostBoundary(), StartsFromInlineEditingHostBoundary()
and EndsByInlineEditingHostBoundary() to avoid unexpected regressions in the
wild.

Depends on D207683

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1f48c21f0456
part 1: Make `WSScanResult` created with `WSType::InUncomposedDoc` if the scan is tried in uncomposed tree r=m_kato
https://hg.mozilla.org/integration/autoland/rev/fa3afe35b103
part 2: Make `WSScanResult` always have non-null `mContent` if it reached something r=m_kato
https://hg.mozilla.org/integration/autoland/rev/6ead0f52b128
part 3: Make `WSScanResult` always have editable element with `mContent` if it reached current block boundary r=m_kato
https://hg.mozilla.org/integration/autoland/rev/218ae793cdd8
part 4: Add `WSType::InlineEditingHostBoundary` r=m_kato

Comment 6

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/1f48c21f0456
https://hg.mozilla.org/mozilla-central/rev/fa3afe35b103
https://hg.mozilla.org/mozilla-central/rev/6ead0f52b128
https://hg.mozilla.org/mozilla-central/rev/218ae793cdd8