Closed Bug 1688832 Opened 2 years ago Closed 2 years ago

Fortify code around `AccessibleCaretManager::UpdateCarets`

Categories

(Core :: DOM: Serializers, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: mbrodesser, Assigned: mbrodesser)

References

(Blocks 1 open bug)

Details

Attachments

(10 files, 11 obsolete files)

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

Could you set proper priority for this bug and bug 1674763?

Flags: needinfo?(mbrodesser)
Severity: -- → N/A
Flags: needinfo?(mbrodesser)
Priority: -- → P2

Will help to simplify AccessibleCaretManager::Dispatch....

Attachment #9199470 - Attachment description: Bug 1688832: part 29) Encapsulate `AccessibleCaretManager::mFirstCaret`, `mSecondCaret` in `mCarets`. r=TYLin → Bug 1688832: part 1) Encapsulate `AccessibleCaretManager::mFirstCaret`, `mSecondCaret` in `mCarets`. r=TYLin
Attachment #9199471 - Attachment description: Bug 1688832: part 30) Add `AccessibleCaretManager::Carets::AreLogicallyVisible`. r=TYLin → Bug 1688832: part 2) Add `AccessibleCaretManager::Carets::AreLogicallyVisible`. r=TYLin
Attachment #9199472 - Attachment description: Bug 1688832: part 31) Add `AccessibleCaretManager::Carets::AreVisuallyVisible` r=TYLin → Bug 1688832: part 3) Add `AccessibleCaretManager::Carets::AreVisuallyVisible` r=TYLin
Attachment #9199473 - Attachment description: Bug 1688832: part 32) Add `AccessibleCaretManager::SelectionStringifyer`. r=TYLin → Bug 1688832: part 4) Add `AccessibleCaretManager::SelectionStringifyer`. r=TYLin
Attachment #9199474 - Attachment description: Bug 1688832: part 33) Add `static` `AccessibleCaretManager::GetSelection`, `::GetFrameSelection`. r=TYLin → Bug 1688832: part 5) Add `static` `AccessibleCaretManager::GetSelection`, `::GetFrameSelection`. r=TYLin
Attachment #9199475 - Attachment description: Bug 1688832: part 34) Prepare `AccessibleCaretManager::DispatchCaretStateChangedEvent` for splitting. r=TYLin → Bug 1688832: part 6) Prepare `AccessibleCaretManager::DispatchCaretStateChangedEvent` for splitting. r=TYLin
Attachment #9199476 - Attachment description: Bug 1688832: part 35) Add `AccessibleCaretManager::Carets::GetFirst`. r=TYLin → Bug 1688832: part 7) Add `AccessibleCaretManager::Carets::GetFirst`. r=TYLin
Attachment #9199477 - Attachment description: Bug 1688832: part 36) Add `AccessibleCaretManager::Carets::GetSecond`. r=TYLin → Bug 1688832: part 8) Add `AccessibleCaretManager::Carets::GetSecond`. r=TYLin
Attachment #9199478 - Attachment description: Bug 1688832: part 37) Add non-default constructor to `AccessibleCaretManager::Carets`. r=TYLin → Bug 1688832: part 9) Add non-default constructor to `AccessibleCaretManager::Carets`. r=TYLin
Attachment #9199479 - Attachment description: Bug 1688832: part 38) Hide `UniquePtr`s in `AccessibleCaretManager::Carets`. r=TYLin → Bug 1688832: part 10) Hide `UniquePtr`s in `AccessibleCaretManager::Carets`. r=TYLin
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0848f7977679
part 1) Encapsulate `AccessibleCaretManager::mFirstCaret`, `mSecondCaret` in `mCarets`. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/c190058d85b8
part 2) Add `AccessibleCaretManager::Carets::AreLogicallyVisible`. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/bd1ad0a8f943
part 3) Add `AccessibleCaretManager::Carets::AreVisuallyVisible` r=TYLin
https://hg.mozilla.org/integration/autoland/rev/dcc61c66c659
part 4) Add `AccessibleCaretManager::SelectionStringifyer`. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/766c328fa158
part 5) Add `static` `AccessibleCaretManager::GetSelection`, `::GetFrameSelection`. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/7183b7f948a6
part 6) Prepare `AccessibleCaretManager::DispatchCaretStateChangedEvent` for splitting. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/a52ca43f4156
part 7) Add `AccessibleCaretManager::Carets::GetFirst`. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/d836c47aef1c
part 8) Add `AccessibleCaretManager::Carets::GetSecond`. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/5b09f705ef8d
part 9) Add non-default constructor to `AccessibleCaretManager::Carets`. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/595e1c877517
part 10) Hide `UniquePtr`s in `AccessibleCaretManager::Carets`. r=TYLin

Thanks for ni?-request. Looking into it.

Flags: needinfo?(mbrodesser)
Attachment #9199470 - Attachment is obsolete: true
Attachment #9199471 - Attachment is obsolete: true
Attachment #9199472 - Attachment is obsolete: true
Attachment #9199473 - Attachment is obsolete: true
Attachment #9199474 - Attachment is obsolete: true
Attachment #9199475 - Attachment is obsolete: true
Attachment #9199476 - Attachment is obsolete: true
Attachment #9199477 - Attachment is obsolete: true
Attachment #9199478 - Attachment is obsolete: true
Attachment #9199479 - Attachment is obsolete: true

Helps to see that they're never replaced with new instances.

Depends on D103312

Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b674e6e08a46
part 0) Delete copy-constructor and copy-assignment operator of `AccessibleCaretManager::LayoutFlusher`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/8a998edf7b80
part 1) Encapsulate `AccessibleCaretManager::mFirstCaret`, `mSecondCaret` in `mCarets`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/305827a1d598
part 2) Add `AccessibleCaretManager::Carets::HasLogicallyVisibleCaret`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/881e74658476
part 3) Add `AccessibleCaretManager::Carets::HasVisuallyVisibleCaret`. r=smaug
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2533d3db342a
part 4) Add `AccessibleCaretManager::Carets::GetFirst`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/520af9cbedbe
part 5) Add `AccessibleCaretManager::Carets::GetSecond`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/8caecd73211a
part 6) Hide `AccessibleCaretManager::Carets::mFirst`, `mSecond`. r=smaug
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5fd34b0e1406
part 7) Declare `AccessibleCaretManager::GetAllChildFrameRectsUnion` `static`. r=TYLin
Attachment #9200361 - Attachment is obsolete: true
Attachment #9200362 - Attachment description: Bug 1688832: part 9) Declare `nsFrameSelection::TakeFocus` `[[nodiscard]]`. r=smaug,TYLin → Bug 1688832: part 8) Declare `nsFrameSelection::TakeFocus` `[[nodiscard]]`. r=smaug,TYLin
Pushed by mbrodesser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/706db2f38fcc
part 8) Declare `nsFrameSelection::TakeFocus` `[[nodiscard]]`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/ca8fdec1a450
part 9) Change argument of `nsFrameSelection::TakeFocus` from pointer to reference. r=smaug

More work is required to fortify the code, but that'll happen in a separate ticket.

Status: NEW → RESOLVED
Closed: 2 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.