1623858 - Document and simplify some Selection related code

Status: RESOLVED FIXED

(Core :: DOM: Selection, enhancement)

Comment 1

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 1) Rename `nsINode::IsSelectionDescendant` to `nsINode::IsMaybeSelected`. r=smaug

More accurate.

Comment 2

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 2) Declare `nsIContent::HasIndependentSelection` `const`. r=smaug

Depends on D67599

Comment 3

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 3) Remove `TextInputSelectionController::mLimiter`. r=smaug

It was never read apart from forwarding it to nsFrameSelection.

Depends on D67600

Comment 4

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 4) Document `PresShell::mSelectionFlags`. r=smaug

Depends on D67601

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/84477c862580
part 1) Rename `nsINode::IsSelectionDescendant` to `nsINode::IsMaybeSelected`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/fc611a718497
part 2) Declare `nsIContent::HasIndependentSelection` `const`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/2caa882e6503
part 3) Remove `TextInputSelectionController::mLimiter`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/d03f4d08e7e7
part 4) Document `PresShell::mSelectionFlags`. r=smaug

Comment 6

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 5) Document `nsTextFrame::GetSelectionStatus`. r=smaug

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/84477c862580
https://hg.mozilla.org/mozilla-central/rev/fc611a718497
https://hg.mozilla.org/mozilla-central/rev/2caa882e6503
https://hg.mozilla.org/mozilla-central/rev/d03f4d08e7e7

Comment 8

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 6) Use `switch` statement in `nsTextPaintStyle::InitSelectionColorsAndShadow`. r=smaug

Depends on D67644

Comment 9

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 7) Rename and simplify `nsFrame::DisplaySelection`. r=smaug

Depends on D67806

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9c035aabedb4
part 5) Document `nsTextFrame::GetSelectionStatus`. r=smaug

Comment 11

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 8) Document `nsIFrame::ComputeSelectionStyle`. r=smaug

Depends on D67807

Comment 12

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 9) Document more variables corresponding to `nsISelectionController::getDisplaySelection`. r=smaug

Depends on D67846

Comment 13

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 10) Simplify `Selection::AddRangesForSelectableNodes`'s signature. r=smaug

Depends on D67847

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e0d8e21f9f0
part 6) Use `switch` statement in `nsTextPaintStyle::InitSelectionColorsAndShadow`. r=smaug

Comment 15

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/9c035aabedb4

Comment 16

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/5e0d8e21f9f0

Comment 17

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 11) Split `Selection::AddRangesForSelectableNodes`. r=smaug

Depends on D67848

Comment 18

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 12) Factor determining "selectstart" event target out. r=smaug

Depends on D68008

Comment 19

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 13) Declare some methods in `HyperTextAccessible` `const`. r=smaug

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df14c69672f0
part 7) Rename and simplify `nsFrame::DisplaySelection`. r=smaug

Comment 21

[Oana Pop-Rus]

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

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a460e00bd6d
part 8) Document `nsIFrame::ComputeSelectionStyle`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/70e7b2baa28f
part 9) Document more variables corresponding to `nsISelectionController::getDisplaySelection`. r=smaug

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e1333796925
part 10) Simplify `Selection::AddRangesForSelectableNodes`'s signature. r=smaug

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4ca6d27aa78
part 11) Split `Selection::AddRangesForSelectableNodes`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/3067255d5eb2
part 12) Factor determining "selectstart" event target out. r=smaug
https://hg.mozilla.org/integration/autoland/rev/0c829e76b0c6
part 13) Declare some methods in `HyperTextAccessible` `const`. r=smaug

Comment 25

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/8a460e00bd6d
https://hg.mozilla.org/mozilla-central/rev/70e7b2baa28f

Comment 26

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/7e1333796925
https://hg.mozilla.org/mozilla-central/rev/e4ca6d27aa78
https://hg.mozilla.org/mozilla-central/rev/3067255d5eb2
https://hg.mozilla.org/mozilla-central/rev/0c829e76b0c6

Comment 27

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 14) Incorporate feedback of part 12. r=smaug

Forgot to update the review on Phabricator.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3f2d9592869
part 14) Incorporate feedback of part 12. r=smaug

Comment 29

[Noemi Erli[:noemi_erli]]

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

Comment 30

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 15) Remove some conditions in `nsFrameSelection::TakeFocus` which are always true. r=hsivonen

Comment 31

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 16) Annotate `nsFrameSelection::HandleTableSelection` with `[[nodiscard]]`. r=hsivonen

Depends on D68575

Comment 32

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 17) Assert accessible caret methods in `Selection` are only called for `eNormal` Selections. r=hsivonen

Depends on D68576

Comment 33

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 18) Assert `Selection::StopAutoScrollTimer` is only called for `eNormal` Selections. r=hsivonen

Depends on D68577

Comment 34

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 19) Remove superfluous `nsAutoScrollTimer::Init`. r=hsivonen

Depends on D68578

Comment 35

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 20) Assert `Selection::NotifyAutoCopy` is only called for `eNormal` Selections. r=hsivonen

Depends on D68579

Comment 36

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 21) Add some const correctness to `nsDocumentEncoder::SerializeSelection`. r=hsivonen

Depends on D68580

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69539461f67e
part 15) Remove some conditions in `nsFrameSelection::TakeFocus` which are always true. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/1aa8cbe35ebb
part 16) Annotate `nsFrameSelection::HandleTableSelection` with `[[nodiscard]]`. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/aabd5a62e61c
part 17) Assert accessible caret methods in `Selection` are only called for `eNormal` Selections. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/dd3db84e0787
part 18) Assert `Selection::StopAutoScrollTimer` is only called for `eNormal` Selections. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/d2322ca0888a
part 19) Remove superfluous `nsAutoScrollTimer::Init`. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/a76c67117050
part 20) Assert `Selection::NotifyAutoCopy` is only called for `eNormal` Selections. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/7e158fb43cad
part 21) Add some const correctness to `nsDocumentEncoder::SerializeSelection`. r=hsivonen

Comment 38

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 22) Move focusing common editing host to `Selection::StyledRanges`. r=masayuki

Simplifies understanding Selection::NotifySelectionListeners.

Comment 39

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/69539461f67e
https://hg.mozilla.org/mozilla-central/rev/1aa8cbe35ebb
https://hg.mozilla.org/mozilla-central/rev/aabd5a62e61c
https://hg.mozilla.org/mozilla-central/rev/dd3db84e0787
https://hg.mozilla.org/mozilla-central/rev/d2322ca0888a
https://hg.mozilla.org/mozilla-central/rev/a76c67117050
https://hg.mozilla.org/mozilla-central/rev/7e158fb43cad

Comment 40

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 23) Factor dispatching `selectstart` event out. r=masayuki

Simplifies its caller.

Comment 41

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 24) Use local `doc` variable. r=masayuki

Depends on D68960

Comment 42

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 25) Factor `AreUserSelectedRangesNonEmpty` out. r=masayuki

Depends on D68961

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13865519a6e9
part 22) Move focusing common editing host to `Selection::StyledRanges`. r=masayuki

Comment 44

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9033e89239a7
part 23) Factor dispatching `selectstart` event out. r=masayuki
https://hg.mozilla.org/integration/autoland/rev/fe77c7e6dda4
part 24) Use local `doc` variable. r=masayuki

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/96ca51d3b909
part 25) Factor `AreUserSelectedRangesNonEmpty` out. r=masayuki

Comment 46

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/13865519a6e9
https://hg.mozilla.org/mozilla-central/rev/9033e89239a7
https://hg.mozilla.org/mozilla-central/rev/fe77c7e6dda4
https://hg.mozilla.org/mozilla-central/rev/96ca51d3b909

Comment 47

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1623858: part 26) Annotate `MaybeAddRangeAndTruncateOverlaps` with `MOZ_CAN_RUN_SCRIPT`. r=masayuki

Comment 48

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39c389ff5ce2
part 26) Annotate `MaybeAddRangeAndTruncateOverlaps` with `MOZ_CAN_RUN_SCRIPT`. r=masayuki

Comment 50

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/39c389ff5ce2