1540037 - Move `TextEditor` methods which are used with `HTMLEditor` instances for splitting TextEditor and HTMLEditor classes

Status: RESOLVED FIXED

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

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 1: Move `EditorBase::ReplaceContainerWithTransaction()` and related methods to `HTMLEditor` r=m_kato!

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

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 2: Move `EditorBase::RemoveContainerWithTransaction()` to `HTMLEditor` r=m_kato!

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

Depends on D72822

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 3: Move `EditorBase::InsertContainerWithTransaction()` and related methods to `HTMLEditor` r=m_kato!

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

Depends on D72823

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 4: Move `EditorBase::SplitNodeDeepWithTransaction()` to `HTMLEditor` r=m_kato!

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

Depends on D72825

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 5: Move `EditorBase::JoinNodesWithTransaction()` to `HTMLEditor` r=m_kato!

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

Depends on D72826

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 6: Move `TextEditor::DeleteSelectionAndCreateElement()` and `TextEditor::DeleteSelectionAndPrepareToCreateNode()` to `HTMLEditor` r=m_kato!

Both of them are used only by HTMLEditor so that we can move them to
HTMLEditor.

Depends on D72829

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 7: Move `EditorBase::PrepareToInsertBRElement()` r=m_kato!

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

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 8: Move `EditorBase::SplitNodeWithTransaction()` to `HTMLEditor` r=m_kato!

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

Depends on D72831

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 9: Move `EditorBase::DoSplitNode()` to `HTMLEditor` r=m_kato!

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

Depends on D72832

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 10: Move `EditorBase::DoJoinNodes()` to `HTMLEditor` r=m_kato!

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

Depends on D72833

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 11: Move `EditorBase::MoveAllChilren()` to `HTMLEditor` r=m_kato!

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

Depends on D72835

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 12: Move `EditorBase::MovePreviousSiblings()` to `HTMLEditor` r=m_kato!

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

Depends on D72836

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 13: Move `EditorBase::MoveChildren()` to `HTMLEditor` r=m_kato!

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

Depends on D72844

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 14: Move `EditorBase::MoveNodeWithTransaction()` and `EditorBase::MoveNodeToEndWithTransaction()` r=m_kato!

They are used only by HTMLEditor so that we can move them.

Depends on D72845

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 15: Rename `HTMLEditor::MoveChildren()` and `HTMLEditor::MoveNodeOrChildren()` to `*WithTransaction()` r=m_kato!

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2f1fad33d99
part 1: Move `EditorBase::ReplaceContainerWithTransaction()` and related methods to `HTMLEditor` r=m_kato

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d6a086d101a
part 2: Move `EditorBase::RemoveContainerWithTransaction()` to `HTMLEditor` r=m_kato

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3741ec9234d0
part 3: Move `EditorBase::InsertContainerWithTransaction()` and related methods to `HTMLEditor` r=m_kato

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ba97dfc2ddd
part 4: Move `EditorBase::SplitNodeDeepWithTransaction()` to `HTMLEditor` r=m_kato

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff18fa29d992
part 5: Move `EditorBase::JoinNodesWithTransaction()` to `HTMLEditor` r=m_kato

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/705c53101ce1
part 6: Move `TextEditor::DeleteSelectionAndCreateElement()` and `TextEditor::DeleteSelectionAndPrepareToCreateNode()` to `HTMLEditor` r=m_kato

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69e74ec4f750
part 7: Move `EditorBase::PrepareToInsertBRElement()` r=m_kato

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb080a8b68e9
part 8: Move `EditorBase::SplitNodeWithTransaction()` to `HTMLEditor` r=m_kato

Comment 24

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e2f1fad33d99
https://hg.mozilla.org/mozilla-central/rev/1d6a086d101a
https://hg.mozilla.org/mozilla-central/rev/3741ec9234d0
https://hg.mozilla.org/mozilla-central/rev/1ba97dfc2ddd

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33c98b169ef2
part 9: Move `EditorBase::DoSplitNode()` to `HTMLEditor` r=m_kato

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/004a3f6df7d9
part 10: Move `EditorBase::DoJoinNodes()` to `HTMLEditor` r=m_kato

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ea49a36a809
part 11: Move `EditorBase::MoveAllChilren()` to `HTMLEditor` r=m_kato

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ab6dfbeffce
part 12: Move `EditorBase::MovePreviousSiblings()` to `HTMLEditor` r=m_kato

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5febc73cdbd1
part 13: Move `EditorBase::MoveChildren()` to `HTMLEditor` r=m_kato

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b68862b930cd
part 14: Move `EditorBase::MoveNodeWithTransaction()` and `EditorBase::MoveNodeToEndWithTransaction()` r=m_kato

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9be5da1e7893
part 15: Rename `HTMLEditor::MoveChildren()` and `HTMLEditor::MoveNodeOrChildren()` to `*WithTransaction()` r=m_kato

Comment 32

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/ff18fa29d992
https://hg.mozilla.org/mozilla-central/rev/705c53101ce1
https://hg.mozilla.org/mozilla-central/rev/69e74ec4f750
https://hg.mozilla.org/mozilla-central/rev/eb080a8b68e9
https://hg.mozilla.org/mozilla-central/rev/33c98b169ef2
https://hg.mozilla.org/mozilla-central/rev/004a3f6df7d9
https://hg.mozilla.org/mozilla-central/rev/9ea49a36a809
https://hg.mozilla.org/mozilla-central/rev/3ab6dfbeffce
https://hg.mozilla.org/mozilla-central/rev/5febc73cdbd1
https://hg.mozilla.org/mozilla-central/rev/b68862b930cd
https://hg.mozilla.org/mozilla-central/rev/9be5da1e7893

Comment 33

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:jstutte, maybe it's time to close this bug?

Comment 34

[Jens Stutte [:jstutte]]

:masayuki, is the leave-open here still meaningful? What is the remaining work to do?

Comment 35

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Still there are a lot of methods which should be moved to another class, for example, from TextEditor to EditorBase. Such methods block to make HTMLEditor stop inheriting TextEditor.

Comment 36

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The list of TextEditor methods.
https://docs.google.com/spreadsheets/d/1QO6FljlIMOaSDxTQrNJuwBHbI8ZToDw0s_328LXEolQ/edit?usp=sharing

Some of them should be moved explicitly, but I'm not sure for some others of them.

Comment 37

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 16: Move composition event handlers from `TextEditor` to `EditorBase` r=m_kato!

IME is available in both TextEditor and HTMLEditor, and the handling
code is almost same (they partially do different things with checking
IsHTMLEditor()). Therefore, we should move them to EditorBase for
making HTMLEditor possible to inherit only EditorBase in the future.

Comment 38

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 17: Move `TextEditor::OnInputText()` into `EditorBase` r=m_kato!

This method is semi-public method, meaning that this is commonly used by
public methods which handle various user input and that causes inputting
text, both in TextEditor and HTMLEditor.

Therefore, for making HTMLEditor stop inheriting TextEditor class in the
future, we should move it into EditorBase.

Depends on D115784

Comment 39

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 18: Move `ReplaceTextAsAction()` and `ReplaceSelectionAsSubAction()` to `EditorBase` r=m_kato!

They are used by setting text value of TextEditor or replacing a misspelled
word with a new word in both TextEditor and HTMLEditor. Therefore,
they should be in the EditorBase rather than TextEditor.

Note that the path of the former case may be in a hot path. Therefore, we need
to keep redirecting to TextEditor for keeping the performance only in the
case.

Depends on D115785

Comment 40

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 19: Make `IsEmpty()` be a virtual method of `EditorBase` and implement `nsIEditor::GetDocumentIsEmpty()` in `EditorBase` r=m_kato!

Currently, EditorBase::GetDocumentIsEmpty() is implemented by TextEditor,
and it refers only IsEmpty() which is implemented both by TextEditor and
HTMLEditor. So, IsEmpty() should be a virtual method of EditorBase,
then, EditorBase can implement GetDocumentIsEmpty().

Depends on D115786

Comment 41

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 20: Move `UndoAsAction()` and `RedoAsAction()` from `TextEditor` to `EditorBase` r=m_kato!

They just work with a transaction manager and transactions, and they are used
by both TextEditor and HTMLEditor. Therefore, they should be in
EditorBase for making HTMLEditor stop inheriting TextEditor in the
future.

Depends on D115787

Comment 42

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 21: Move `TextEditor::IsCopyToClipboardAllowed()` to `EditorBase` r=m_kato!

It calls only its helper method, and the helper method,
IsCopyToClipboardAllowedInternal(), needs to check complicated things only
when it's a password editor.

This patch makes IsCopyToCLipboardAllowedInternal() a virtual method. But
this shouldn't cause performance regression because this should be called
only when updating menu items or handling "copy" commands. So, this shouldn't
be in any hot paths.

Depends on D115788

Comment 43

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 22: Move `IsCopyCommandEnabled()` and `IsCutCommandEnabled()` from `TextEditor` to `HTMLEditor` r=m_kato!

Of course, they are used for HTMLEditor instances too. Therefore, they
should be in EditorBase rather than TextEditor.

Depends on D115789

Comment 44

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 23: Move `TextEditor::AreClipboardCommandsUnconditionallyEnabled()` into `EditorBase` r=m_kato!

This is used by HTMLEditor too. So, this should be a method of EditorBase.

Depends on D115790

Comment 45

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 24: Move `TextEditor::FireClipboardEvent()` to `EditorBase` r=m_kato!

It's used by HTMLEditor too. So, we should move it into EditorBase.

Depends on D115791

Comment 46

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 25: Move `TextEditor::CutAsAction()` to `EditorBase` r=m_kato!

It's a common method to cut selection. Therefore, it should be in EditorBase.

Depends on D115792

Comment 47

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 26: Get rid of `TextEditor::Copy()` r=m_kato!

It's common method of TextEditor and HTMLEditor, but implemented by
TextEditor even though it's an override of nsIEditor's method.

Therefore, it should be implemented in EditorBase instead.

Depends on D115793

Comment 48

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 27: Move `TextEditor::CanDeleteSelection()` to `EditorBase` r=m_kato!

It's used with both TextEditor and HTMLEditor instances. So, it should be
implemented in EditorBase instead.

Depends on D115794

Comment 49

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 28: Add pure virtual methods to `EditorBase`, which are originated from `TextEditor` r=m_kato!

TextEditor declares some virtual methods newly. However, for moving some
methods from TextEditor to EditorBase, they should be accessible from
EditorBase. Therefore, this patch adds declarations of abstract virtual
methods of them to EditorBase.

Depends on D115795

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7e4abc8057a
part 16: Move composition event handlers from `TextEditor` to `EditorBase` r=m_kato

Comment 51

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d3b833e084b
part 17: Move `TextEditor::OnInputText()` into `EditorBase` r=m_kato

Comment 52

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d9084863822
part 18: Move `ReplaceTextAsAction()` and `ReplaceSelectionAsSubAction()` to `EditorBase` r=m_kato

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8f1b67bbc5af
part 19: Make `IsEmpty()` be a virtual method of `EditorBase` and implement `nsIEditor::GetDocumentIsEmpty()` in `EditorBase` r=m_kato

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08b0dccfddf7
part 20: Move `UndoAsAction()` and `RedoAsAction()` from `TextEditor` to `EditorBase` r=m_kato

Comment 55

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85911d6eabbb
part 21: Move `TextEditor::IsCopyToClipboardAllowed()` to `EditorBase` r=m_kato

Comment 56

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b48dcb60529
part 22: Move `IsCopyCommandEnabled()` and `IsCutCommandEnabled()` from `TextEditor` to `HTMLEditor` r=m_kato

Comment 57

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/c7e4abc8057a
https://hg.mozilla.org/mozilla-central/rev/6d3b833e084b
https://hg.mozilla.org/mozilla-central/rev/9d9084863822
https://hg.mozilla.org/mozilla-central/rev/8f1b67bbc5af

Comment 58

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d14c17436d81
part 23: Move `TextEditor::AreClipboardCommandsUnconditionallyEnabled()` into `EditorBase` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/0faf5e8f142b
part 24: Move `TextEditor::FireClipboardEvent()` to `EditorBase` r=m_kato

Comment 59

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/08b0dccfddf7
https://hg.mozilla.org/mozilla-central/rev/85911d6eabbb
https://hg.mozilla.org/mozilla-central/rev/0b48dcb60529

Comment 60

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 29: Move `nsIEditor.documentCharacterSet` definitions into `HTMLEditor` r=m_kato!

The attribute is used only with HTMLEditor, and it does not make sense to
access document's character-set via TextEditor. Therefore, this patch
makes it implement in HTMLEditor (EditorBase will return
NS_ERROR_NOT_AVAILABLE both getter and setter).

Note that EditorBase::GetDocumentCharsetInternal() is required by
TextEditor::ComputeValueInternal(). Therefore, it needs to stay in
EditorBase.

Depends on D115796

Comment 61

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d6956046a4e
part 25: Move `TextEditor::CutAsAction()` to `EditorBase` r=m_kato

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1bf9011c27c2
part 26: Get rid of `TextEditor::Copy()` r=m_kato

Comment 63

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16ece4ec2890
part 27: Move `TextEditor::CanDeleteSelection()` to `EditorBase` r=m_kato

Comment 64

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adb0d91643e6
part 28: Add pure virtual methods to `EditorBase`, which are originated from `TextEditor` r=m_kato

Comment 65

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3c85e6dc75bf
part 29: Move `nsIEditor.documentCharacterSet` definitions into `HTMLEditor` r=m_kato

Comment 66

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/d14c17436d81
https://hg.mozilla.org/mozilla-central/rev/0faf5e8f142b
https://hg.mozilla.org/mozilla-central/rev/2d6956046a4e
https://hg.mozilla.org/mozilla-central/rev/1bf9011c27c2
https://hg.mozilla.org/mozilla-central/rev/16ece4ec2890
https://hg.mozilla.org/mozilla-central/rev/adb0d91643e6

Comment 67

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/3c85e6dc75bf

Comment 68

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 30: Get rid of `TextEditor::SharedOutputString()` r=m_kato!

It's now used only by HTMLEditor::Rewrap() and it does simple thing with
TextEditor::ComputeValueInternal(). So, we can make Rewrap() directly
do it instead.

Comment 69

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 31: Move `TextEditor::GetAndIniitDocEncoder()` into `EditorBase` r=m_kato!

It's a helper method of TextEditor::ComputeValueInternal() which is used by
TextEditor and HTMLEditor::Rewrap(). So, before we move
ComputeValueInternal(), we need to move this first.

Depends on D116347

Comment 70

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 32: Move `TextEditor::ComputeValueInternal()` into `EditorBase` r=m_kato!

Depends on D116348

Comment 71

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 33: Implement `nsIEditor::OutputToString()` in `EditorBase` rather than `TextEditor` r=m_kato!

It's used both with TextEditor instance and HTMLEditor instance. So, it
should be implemented in EditorBase.

Depends on D116349

Comment 72

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 34: Create `HandleKeyPressEventInReadOnlyMode()` r=m_kato!

In any types of editor, EditorBase handles if it's in the read-only mode.
For making HandleKeyPressEvent relation between classes simpler, let's
handle it in an independent method.

Depends on D116350

Comment 73

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 35: Make `TextEditor` and `HTMLEditor` handle modifier keys' `eKeyPress` event directly r=m_kato!

Such events shouldn't be fired, but for now, we should make them handled in
the overrides. Then, we can avoid the skipping TextEditor case from
HTMLEditor.

Depends on D116351

Comment 74

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 36: Make `EditorBase` handle `Delete`, `Backspace` and `Tab` of plaintext editor mode r=m_kato!

Delete and Backspace keys are handled by same code. So, the code should
be in EditorBase instead of TextEditor.

If HTMLEditor is in the plaintext editing mode of mail composer, Tab key
is also handled by the same code as TextEditor. So, the code in TextEditor
should be moved to EditorBase too and HTMLEditor should call EditorBase's
method only when it's in the plaintext mode.

Depends on D116352

Comment 75

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/219a530441fd
part 30: Get rid of `TextEditor::SharedOutputString()` r=m_kato

Comment 76

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99e98ac9c89c
part 31: Move `TextEditor::GetAndIniitDocEncoder()` into `EditorBase` r=m_kato

Comment 77

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d28dcb2d4d4
part 32: Move `TextEditor::ComputeValueInternal()` into `EditorBase` r=m_kato

Comment 78

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/219a530441fd

Comment 79

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ccd13d2a6fd
part 33: Implement `nsIEditor::OutputToString()` in `EditorBase` rather than `TextEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/4c550655dda5
part 34: Create `HandleKeyPressEventInReadOnlyMode()` r=m_kato

Comment 80

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e902ab5efc57
part 35: Make `TextEditor` and `HTMLEditor` handle modifier keys' `eKeyPress` event directly r=m_kato

Comment 81

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a438fd03bdb8
part 36: Make `EditorBase` handle `Delete`, `Backspace` and `Tab` of plaintext editor mode r=m_kato

Comment 82

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/99e98ac9c89c
https://hg.mozilla.org/mozilla-central/rev/9d28dcb2d4d4

Comment 83

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7ccd13d2a6fd
https://hg.mozilla.org/mozilla-central/rev/4c550655dda5
https://hg.mozilla.org/mozilla-central/rev/e902ab5efc57
https://hg.mozilla.org/mozilla-central/rev/a438fd03bdb8

Comment 84

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 37: Move `TextEditor::PrepareToInsertContent()` into `EditorBase` r=m_kato!

Depends on D116541

Comment 85

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 38: Move `TextEditor::InsertTextAt()` into `EditorBase` r=m_kato!

Depends on D116564

Comment 86

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 39: Move `TextEditor::IsSafeToInsertData()` into `EditorBase` r=m_kato!

Depends on D116565

Comment 87

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 40: Move `TextEditor::PrepareTransferable()` into `EditorUtils` r=m_kato!

It just creates an nsITransferable instance and add 2 flavors for storing
plain text. Therefore, it can be in EditorUtils instead.

Depends on D116566

Comment 88

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 41: Split `TextEditor::PasteAsAction()` r=m_kato!

It should be a virtual method derived from EditorBase, and TextEditor
and HTMLEditor should override it. Then, nsIEditor::Paste() requires
referring vtable again if we keep implementing it only in EditorBase.
Therefore, this patch avoid it with implementing it in both TextEditor
and HTMLEditor.

Depends on D116567

Comment 89

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 42: Move `TextEditor::DeleteSelectionByDragAsAction()` into `EditorBase` r=m_kato!

Depends on D116568

Comment 90

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e61485ac614
part 37: Move `TextEditor::PrepareToInsertContent()` into `EditorBase` r=m_kato

Comment 91

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52b13b5f2c4d
part 38: Move `TextEditor::InsertTextAt()` into `EditorBase` r=m_kato

Comment 92

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7beac38583d0
part 39: Move `TextEditor::IsSafeToInsertData()` into `EditorBase` r=m_kato

Comment 93

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9c0c941ca36
part 40: Move `TextEditor::PrepareTransferable()` into `EditorUtils` r=m_kato

Comment 94

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dcac769feba1
part 41: Split `TextEditor::PasteAsAction()` r=m_kato

Comment 95

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 43: Move inserting dropped items part on `HTMLEditor` to `HTMLEditor` r=m_kato!

TextEditor::OnDrop() handles both cases, in TextEditor and in HTMLEditor
because the common part is too complicated to duplicate. However, most
different part is inserting the dropped items part. So, let's make them
into a virtual method.

In this patch, creating a method only in HTMLEditor and moves the part
into it.

Depends on D116569

Comment 96

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 44: Move the inserting dropped items part of `TextEditor::OnDrop()` to the new virtual method r=m_kato!

This patch makes the previously created handler method to virtual (derived from
EditorBase), and makes TextEditor override it.

Depends on D116801

Comment 97

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 45: Move `TextEditor::OnDrop()` into `EditorBase` r=m_kato!

Depends on D116802

Comment 98

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ada44a617ae
part 42: Move `TextEditor::DeleteSelectionByDragAsAction()` into `EditorBase` r=m_kato

Comment 99

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/0e61485ac614
https://hg.mozilla.org/mozilla-central/rev/52b13b5f2c4d
https://hg.mozilla.org/mozilla-central/rev/7beac38583d0
https://hg.mozilla.org/mozilla-central/rev/f9c0c941ca36
https://hg.mozilla.org/mozilla-central/rev/dcac769feba1
https://hg.mozilla.org/mozilla-central/rev/7ada44a617ae

Comment 100

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ca825772b7b
part 43: Move inserting dropped items part on `HTMLEditor` to `HTMLEditor` r=m_kato

Comment 101

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0d2966d1b817
part 44: Move the inserting dropped items part of `TextEditor::OnDrop()` to the new virtual method r=m_kato

Comment 102

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f8fa3223188f
part 45: Move `TextEditor::OnDrop()` into `EditorBase` r=m_kato

Comment 103

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/0ca825772b7b
https://hg.mozilla.org/mozilla-central/rev/0d2966d1b817
https://hg.mozilla.org/mozilla-central/rev/f8fa3223188f

Comment 104

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 46: Move `TextEditor::EnsurePaddingBRElementInMultilineEditor()` into `EditorBase` r=m_kato!

It's used by TextEditor and HTMLEditor in the plaintext mode. Therefore,
it should be moved into EditorBase.

Comment 105

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1540037 - part 47: Move `TextEditor::InitEditorContentAndSelection()` to `EditorBase` r=m_kato!

Depends on D117114

Comment 106

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The last 2 patches are the final ones for this bug.

Comment 107

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/446c8ee9a6ba
part 46: Move `TextEditor::EnsurePaddingBRElementInMultilineEditor()` into `EditorBase` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/4173082d8402
part 47: Move `TextEditor::InitEditorContentAndSelection()` to `EditorBase` r=m_kato

Comment 108

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/446c8ee9a6ba
https://hg.mozilla.org/mozilla-central/rev/4173082d8402