Closed Bug 1765018 Opened 2 years ago Closed 2 years ago

Get rid of implicit conversion of `EditorDOMPointBase`

Categories

(Core :: DOM: Editor, task, P3)

task

Tracking

()

RESOLVED FIXED
101 Branch
Tracking Status
firefox101 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

Attachments

(7 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
No description provided.

The implicit copy constructors and operator= makes it harder to realize
that implicit conversion wastes the runtime cost. Therefore, this patch
replaces them with a template method to convert the EditorDOMPointBase type.

Depends on D143817

This avoids unnecessary conversion at the callers' side, although they should be
omitted by the complier.

Depends on D143878

To avoid the unnecessary conversion between EditorDOMPoint and
EditorRawDOMPoint, it should be a template method which can take/return either
EditorDOMPoint or EditorRawDOMPoint.

Depends on D143879

In theory, methods which touch the DOM tree should take EditorDOMPoint rather
than EditorRawDOMPoint. And now, we can return meaningful value if it
succeeded, with Result.

Therefore, this patch makes it return Result<EditorDOMPoint, nsresult> instead
of taking an out argument, and take only EditorDOMPoint rather than taking any
type of EditorDOMPointBase since it's always converted to EditorDOMPoint.

Depends on D143880

Aligning to EditorBase::InsertTextWithTransaction, these methods should return
Result<EditorDOMPoint, nsresult> rather than taking an out param.

Depends on D143881

Using EditorRawDOMPoint in it just wastes conversion cost to EditorDOMPoint.

Depends on D143882

For avoiding to use To<EditorDOMPoint>() etc from temporary object, such
methods should be templated.

Depends on D143883

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/5b708efff09c
part 1: Get rid of implicit conversion between `EditorDOMPointBase` instances r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/f18b4a048939
part 2: Make `EditorBase::Get(Start|End)Point` return `EditorDOMPoint` or `EditorRawDOMPoint` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/8cdcc0cc8380
part 3: Make `EditorBase::FindBetterInsertionPoint` be a template method r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6b1ea907c72d
part 4: Make `EditorBase::InsertTextWithTransaction` treat only `EditorDOMPoint` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/1c84876d94e5
part 5: Make `InsertText()` and `ReplaceText()` of `WhiteSpaceVisibilityKeeper` return `Result<EditorDOMPoint, nsresult>` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/28aaacf1cf54
part 6: Make `TypeInState::OnSelectionChange` stop using `EditorRawDOMPoint` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/48bf136c4911
part 7: Make some methods which return an instance (not reference) of `EditorDOMPointBase` be template methods r=m_kato
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: