Open Bug 1540037 Opened 1 year ago Updated 22 days ago

Split TextEditor and HTMLEditor

Categories

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

enhancement

Tracking

()

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 2 open bugs)

Details

(Keywords: leave-open, Whiteboard: [h2review-noted])

Attachments

(15 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

Currently, we have 3 editor classes.

EditorBase class is an abstract class.

TextEditor class is derived from EditorBase and represents text editor in <input> and <textarea> elements.

HTMLEditor class is derived from TextEditor and represents all editable content in a document, e.g., multiple <div contenteditable> elements or designMode document.

So, even though EditorBase is an abstract class, it's always instanced with TextEditor class.

Therefore, I don't think that we should keep separating them because when we take a look at a handling path, we may need to go EditorBase.cpp and TextEditor.cpp again and again.

If you have some ideas about this issue, let me know.

Priority: -- → P3
Summary: Should we merge EditorBase into TextEditor? → Split TextEditor and HTMLEditor

They are used only by HTMLEditor so that we should hide them from
TextEditor for making it clearer that they are not used by TextEditor.

Note that there are 2 DeleteNodeWithTransaction() in HTMLEditor class.
One is EditorBase's method and the other is HTMLEditor's method.
HTMLEditor's one is check whether the removing node is editable or not,
but in some cases, we need to move non-editable nodes. Therefore, this
patch makes ReplaceContainerWithTransaction() call EditorBase's one
for keeping current behavior.

Depends on D72586

Its users are only HTMLEditor and CSSEditUtils so that we should move it
into HTMLEditor.

Depends on D72822

They are not used by TextEditor so that we should move them into HTMLEditor.

Depends on D72823

It's only used by HTMLEditor so that we can move it.

Depends on D72825

It's used only by HTMLEditor so that we can move it into HTMLEditor.

Depends on D72826

Although this is called by
EditorBase::InsertPaddingBRElementForEmptyLastLineWithTransaction(), it's
mainly designed for HTMLEditor, but used by TextEditor too only when it
puts <br> element to the end of its anonymous <div> element. Therefore,
when TextEditor calls
InsertPaddingBRElementForEmptyLastLineWithTransaction(), aPointToInsert
is not in a text node:
https://searchfox.org/mozilla-central/rev/158bac3df3a1890da55bdb6ffdaf9a7ffc0bfb0a/editor/libeditor/TextEditSubActionHandler.cpp#917-918,920

And it means that PrepareToInsertBRElement() does nothing:
https://searchfox.org/mozilla-central/rev/158bac3df3a1890da55bdb6ffdaf9a7ffc0bfb0a/editor/libeditor/EditorBase.cpp#1561,1569-1570

Therefore, we can move it into HTMLEditor and makes it possible to move
EditorBase::SplitNodeWithTransaction() called by PrepareToInsertBRElement().

Depends on D72830

Now, it's used only by HTMLEditor so that we can move it.

Depends on D72831

And then, SplitNodeTransaction needs to be store HTMLEditor instead of
EditorBase.

Depends on D72832

Then, we can make JoinNodeTransaction store HTMLEditor instead of
EditorBase.

Depends on D72833

It's used only by HTMLEditor so that we can move it.

Depends on D72835

It's used only by HTMLEditor so that we can move it.

Depends on D72836

And this patch renames it to MoveChildrenBetween() for avoiding overload.

Depends on D72844

The other Move*() are not move nodes without transactions. For making
developers to distinguish the difference, this patch appends WithTransaction
postfix like other methods which modify DOM tree with transactions.

Depends on D72846

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/e2f1fad33d99
part 1: Move `EditorBase::ReplaceContainerWithTransaction()` and related methods to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/1d6a086d101a
part 2: Move `EditorBase::RemoveContainerWithTransaction()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3741ec9234d0
part 3: Move `EditorBase::InsertContainerWithTransaction()` and related methods to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/1ba97dfc2ddd
part 4: Move `EditorBase::SplitNodeDeepWithTransaction()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ff18fa29d992
part 5: Move `EditorBase::JoinNodesWithTransaction()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/705c53101ce1
part 6: Move `TextEditor::DeleteSelectionAndCreateElement()` and `TextEditor::DeleteSelectionAndPrepareToCreateNode()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/69e74ec4f750
part 7: Move `EditorBase::PrepareToInsertBRElement()` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/eb080a8b68e9
part 8: Move `EditorBase::SplitNodeWithTransaction()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/33c98b169ef2
part 9: Move `EditorBase::DoSplitNode()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/004a3f6df7d9
part 10: Move `EditorBase::DoJoinNodes()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/9ea49a36a809
part 11: Move `EditorBase::MoveAllChilren()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3ab6dfbeffce
part 12: Move `EditorBase::MovePreviousSiblings()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/5febc73cdbd1
part 13: Move `EditorBase::MoveChildren()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/b68862b930cd
part 14: Move `EditorBase::MoveNodeWithTransaction()` and `EditorBase::MoveNodeToEndWithTransaction()` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/9be5da1e7893
part 15: Rename `HTMLEditor::MoveChildren()` and `HTMLEditor::MoveNodeOrChildren()` to `*WithTransaction()` r=m_kato
Whiteboard: [h2review-noted]
You need to log in before you can comment on or make changes to this bug.