Closed Bug 1415062 Opened 2 years ago Closed 2 years ago

Optimize editor's Selection.Collapse() calls with RawRangeBoundary

Categories

(Core :: DOM: Editor, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Starting from bug 1408544, editor code uses EditorDOMPoint which is a subclass of RawRangeBoundary.  So, if we add Selection::Collapse(const RawRangeBoundary&), we can optimize the cost of Selection::Collapse() since it may not need to compute offset with expensive method, nsINode::IndexOf().
Comment on attachment 8926268 [details]
Bug 1415062 - part 1: Selection should have Collapse(const RawRangeBoundary&) and Collapse(const RawRangeBoundary&, ErrorResult&) for avoiding computing offset of child node in container

https://reviewboard.mozilla.org/r/197512/#review202742


C/C++ static analysis found 0 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`
Comment on attachment 8926269 [details]
Bug 1415062 - part 2: Editor should use Selection::Collapse(const RawRangeBoundary&) as far as possible

https://reviewboard.mozilla.org/r/197514/#review202756


C/C++ static analysis found 0 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`
Comment on attachment 8926268 [details]
Bug 1415062 - part 1: Selection should have Collapse(const RawRangeBoundary&) and Collapse(const RawRangeBoundary&, ErrorResult&) for avoiding computing offset of child node in container

https://reviewboard.mozilla.org/r/197512/#review202874

Please ask a DOM peer to review this instead, thanks.
(I'm happy to review when I have specific knowledge of the Selection/Range code
at hand that can help, otherwise it's better handled by DOM peers.)
Attachment #8926268 - Flags: review?(mats)
Comment on attachment 8926268 [details]
Bug 1415062 - part 1: Selection should have Collapse(const RawRangeBoundary&) and Collapse(const RawRangeBoundary&, ErrorResult&) for avoiding computing offset of child node in container

Smaug, could you review this? Or, is catalinb better for RangeBoundary related API implementation?
Attachment #8926268 - Flags: review?(bugs)
Comment on attachment 8926268 [details]
Bug 1415062 - part 1: Selection should have Collapse(const RawRangeBoundary&) and Collapse(const RawRangeBoundary&, ErrorResult&) for avoiding computing offset of child node in container

https://reviewboard.mozilla.org/r/197512/#review202904
Attachment #8926268 - Flags: review?(bugs) → review+
Comment on attachment 8926269 [details]
Bug 1415062 - part 2: Editor should use Selection::Collapse(const RawRangeBoundary&) as far as possible

https://reviewboard.mozilla.org/r/197514/#review203110
Attachment #8926269 - Flags: review?(m_kato) → review+
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/e3aec6e0fb79
part 1: Selection should have Collapse(const RawRangeBoundary&) and Collapse(const RawRangeBoundary&, ErrorResult&) for avoiding computing offset of child node in container r=smaug
https://hg.mozilla.org/integration/autoland/rev/bda35e3d8ce4
part 2: Editor should use Selection::Collapse(const RawRangeBoundary&) as far as possible r=m_kato
https://hg.mozilla.org/mozilla-central/rev/e3aec6e0fb79
https://hg.mozilla.org/mozilla-central/rev/bda35e3d8ce4
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.