Closed Bug 1635709 Opened 6 months ago Closed 3 months ago

Clean up code around `nsFrameSelection::HandleDrag`

Categories

(Core :: DOM: Selection, enhancement)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: mbrodesser, Assigned: mbrodesser)

References

(Blocks 1 open bug)

Details

Attachments

(30 files, 2 obsolete files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
No description provided.

Selection's nsAutoScrollTimer uses it and it's clearer when the unit
is known.

Depends on D74051

Keywords: leave-open
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2af59ac0778c
part 1) Rename `nsFrameSelection::SetCaretBidiLevel` to `SetCaretBidiLevelAndMaybeSchedulePaint`. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/b8e38d56a744
part 2) Add unit to `nsITimer`'s `init` methods. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/8a2c548a7d28
part 3) Add unit to `Selection::StartAutoScrollTimer`'s delay argument. r=hsivonen

Allows defriending AutoScroller from Selection and removes the
direct dependency of AutoScroller to Selection.

nsFrameSelection has (too) many responsibilites, this adds some
clarification.

Depends on D74383

Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/626d32b5cebe
part 4) Transform `Selection::DoAutoScroll` to `AutoScroller::DoAutoScroll`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/8faefd45845b
part 5) Annotate `Selection::ReplaceAnchorFocusRange` with `MOZ_CAN_RUN_SCRIPT`. r=masayuki
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf41cf75d4ae
part 6) Rename `nsFrameSelection::*DesiredPos` to `*DesiredCaretPos`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/1142ad80e5cc
part 7) Move `nsFrameSelection::SetDesiredCaretPos` and `FetchDesiredCaretPos` to `DesiredCaretPos`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/98a183ae21fe
part 8) Remove superfluously setting `mIsSet` in `nsFrameSelection`'s constructor. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/1ca9c1b2ad31
part 9) Add `DesiredCaretPos::Invalidate`. r=masayuki
Attachment #9147630 - Attachment is obsolete: true
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c86d82544e54
part 10) Factor `IsAnchorRelativeOperation` out from `AutoPrepareFocusRange`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/17c10a7fa75a
part 11) Factor removing generated ranges out from `AutoPrepareFocusRange`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/84d70bc1f5c3
part 12) Factor determining range most distant from anchor out from `AutoPrepareFocusRange`. r=masayuki

Simplifies reasoning about the code when using searchfox.

Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/88e2374f9c43
part 13) Remove stack variable for `Selection::mUserInitiated`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/520e6ddadf14
part 14) Change `nsRange` argument from `const` pointer to `const` reference around table selection. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/5ba7377eca32
part 15) Rename `GetTableSelectionType` to `GetTableSelectionMode`. r=masayuki

The location wasn't used from the caller of
GetTableCellLocationFromRange.

However, GetTableCellLocationFromRange
included flushing frames, this is now done in
HTMLEditor::CellIndexes::Update.

Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4951563fd101
part 16) Replace `GetTableCellLocationFromRange` with `GetTableSelectionMode`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/b933f7ded872
part 17) Annotate `CellIndexes`'s methods with `MOZ_CAN_RUN_SCRIPT`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/6e994c07eca1
part 18) Correct assignment to `aDidAddRange` in `Selection::MaybeAddTableCellRange`. r=masayuki
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/25ef52043c93
part 19) Clean up a little in `nsFrameSelection::MoveCaret`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/2824405e6eb7
part 20) Rename `GetCellParent` to `GetClosestInclusiveTableCellAncestor`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/14275b5ea171
part 21) Rename `TableSelection::mCellParent` to `mClosestInclusiveTableCellAncestor`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/9c52c330dd88
part 22) Rename local `cellparent` variables to `inclusiveTableCellAncestor`. r=masayuki
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d1d29a18bc0b
part 23) Factor code out to `IsContentInActivelyEditableTableCell`. r=masayuki

TableSelection has fewer responsibilites than nsFrameSelection. This
simplifies reasoning about TakeFocus.

HandleSelection was too complex.

The arguments of the new methods will be renamed in a separate commit to
simplify reviewing and avoid mistakes.

Depends on D77252

Attachment #9152396 - Attachment is obsolete: true
Attachment #9152397 - Attachment description: Bug 1635709: part 25.1) Factor `HandleDragSelecting` and `HandleMouseUpOrDown` out from `TableSelection::HandleSelection`. r=masayuki → Bug 1635709: part 24.1) Factor `HandleDragSelecting` and `HandleMouseUpOrDown` out from `TableSelection::HandleSelection`. r=masayuki
Attachment #9152398 - Attachment description: Bug 1635709: part 25.2) Rename `childContent` to `aChild`. r=masayuki → Bug 1635709: part 24.2) Rename `childContent` to `aChildContent`. r=masayuki
Attachment #9152642 - Attachment description: Bug 1635709: part 26) `const` qualify arguments of `GetCellLayout` and `GetCellIndexes`. r=masayuki → Bug 1635709: part 25) `const` qualify arguments of `GetCellLayout` and `GetCellIndexes`. r=masayuki
Attachment #9152643 - Attachment description: Bug 1635709: part 27) Remove some code duplication in `HandleDragSelecting`. r=masayuki → Bug 1635709: part 26) Remove some code duplication in `HandleDragSelecting`. r=masayuki

Simple application of ./mach clang-format -p layout/generic/nsFrameSelection.cpp. Separating this from previous
changes simplified reviewing and decreases the probability for mistakes.

Depends on D77424

Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/70cb19f1ad24
part 24.1) Factor `HandleDragSelecting` and `HandleMouseUpOrDown` out from `TableSelection::HandleSelection`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/423000b06503
part 24.2) Rename `childContent` to `aChildContent`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/df78142de12b
part 25) `const` qualify  arguments of `GetCellLayout` and `GetCellIndexes`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/8568aae413f7
part 26) Remove some code duplication in `HandleDragSelecting`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/cb312ffd7223
part 27) clang-format previous changes in nsFrameSelection.cpp. r=masayuki
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ceadf0ed0f1
part 28) Declare some function arguments in `nsFrameSelection` `const`. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/3acd33c72d73
part 29) Factor finding first and last cell of row or column out. r=masayuki
Keywords: leave-open
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.