1816581 - Make some `nsFrameSelection` methods which consider next/previous position from a point static

Status: RESOLVED FIXED

(Core :: DOM: Selection, enhancement, P3)

Description

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

Currently, nsFrameSelection::CreateRangeExtendedToSomewhere<RangeType>, nsFrameSeleciton::MoveCaret and their helper methods computes new position with instance members. Therefore, it's hard to use them without changing Selection. I think that they should be in new SelectionModifyUtils and split the methods to computing part and modifying part.

Comment 1

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

Oh, this requires bigger patch than I've expected.

nsFrameSelection::PeekOffsetForCaretMove() depends on nsCaretFrame::GetCaretFrameForNodeOffset via Selection::GetPrimaryFrameForFocusNode. I'll put off to work on this for a while.

Comment 2

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

Attachment: Bug 1816581 - part 1: Make `CaretAssociationHint` an enum class r=emilio!,#dom-core

For forward-decl-able.

Comment 3

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

Attachment: Bug 1816581 - part 2: Make `nsCaret::GetCaretFrameForNodeOffset` static r=emilio!,#dom-core

Oddly, it updates nsFrameSelection instance and some root callers may depend
on that. Therefore, this patch keep updating nsFrameSelection in the callers.

Depends on D197283

Comment 4

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

Attachment: Bug 1816581 - part 3: Make `Selection::GetPrimaryOrCaretFrameForNodeOffset` static r=emilio!,#dom-core

Depends on D197284

Comment 5

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

Attachment: Bug 1816581 - part 4: Make `Selection::GetPrimaryFrameForFocusNode` stop updating the caret association hint of `nsFrameSelection` r=emilio!,#dom-core

Its name sounds a getter, but it updates the caret association hint of
nsFrameSelection, but at least, one of its callers,
nsFrameSelection::PeekOffsetForCaretMove shouldn't update it because it
just scans the previous or next position for caret.

Then, Selection::GetPrimaryFrameForFocusNode can be implemented with a
new static method which is useful for PeekOffsetForCaretMove when it becomes
a static method.

Depends on D197285

Comment 6

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

Attachment: Bug 1816581 - part 5: Make `nsFrameSelection::PeekOffsetForCaretMove` implement without members r=emilio!,#dom-core

A caller of it should be independent from nsFrameSelection instance.
Therefore, I'd like to separate it to static method part and instance method
part.

Depends on D197286

Comment 7

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

Attachment: Bug 1816581 - part 6: Move the static methods used for moving caret or considering caret geometry into new utility class r=emilio!,#dom-core

The methods are in nsCaret, nsFrameSelection and dom::Selection. That
makes harder to read, and they are called each other, so, they are reused for
different purpose. Therefore, it must be better to move them into a new class.
Then, the name differences may become more unclear. If you think so, feel free
to request renaming some methods of them.

Depends on D197287

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3ae22226f4d
part 1: Make `CaretAssociationHint` an enum class r=emilio
https://hg.mozilla.org/integration/autoland/rev/b3408c9797ac
part 2: Make `nsCaret::GetCaretFrameForNodeOffset` static r=emilio
https://hg.mozilla.org/integration/autoland/rev/30647b4ef128
part 3: Make `Selection::GetPrimaryOrCaretFrameForNodeOffset` static r=emilio
https://hg.mozilla.org/integration/autoland/rev/de056271d777
part 4: Make `Selection::GetPrimaryFrameForFocusNode` stop updating the caret association hint of `nsFrameSelection` r=emilio
https://hg.mozilla.org/integration/autoland/rev/51faef0a512a
part 5: Make `nsFrameSelection::PeekOffsetForCaretMove` implement without members r=emilio
https://hg.mozilla.org/integration/autoland/rev/2a86b8379416
part 6: Move the static methods used for moving caret or considering caret geometry into new utility class r=emilio

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/d3ae22226f4d
https://hg.mozilla.org/mozilla-central/rev/b3408c9797ac
https://hg.mozilla.org/mozilla-central/rev/30647b4ef128
https://hg.mozilla.org/mozilla-central/rev/de056271d777
https://hg.mozilla.org/mozilla-central/rev/51faef0a512a
https://hg.mozilla.org/mozilla-central/rev/2a86b8379416