1574852 - Move all protected/private methods of TextEditRules/HTMLEditRules to editor class or somewhere

Status: RESOLVED FIXED

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

Description

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

I'm thinking that for keeping each line history in TextEditRules.cpp and HTMLEditRules.cpp, we should make existing methods in them stay in them. And then, they should be renamed to TextEditSubActionHandler.cpp and HTMLEditSubActionHandler.cpp.

In this bug, I'll move protected/private methods in them to EditorBase/TextEditor/HTMLEditor or some utility classes. If the destination is TextEditor or HTMLEditor, I'll keep them staying in the cpp files.

Comment 1

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

This will be longer journey than I expected and becomes risky. So, we shouldn't land all patches once. We should land only several patches every time. Then, it helps to investigate regression point.

Comment 2

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

Attachment: Bug 1574852 - part 1: Get rid of `HTMLEditRules::IsBlockNode()`

HTMLEditRules::IsBlockNode() just wraps HTMLEditor::NodeIsBlockStatic()
and all its users will be moved into HTMLEditor. Therefore, we should
unwrap it.

Comment 3

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

Attachment: Bug 1574852 - part 2: Replace `HTMLEditRules::IsInlineNode()` with `HTMLEditor::NodeIsInlineStatic()`

HTMLEditRules::IsInlineNode() is a wrapper of
HTMLEditor::NodeIsInlineStatic(), but returns opposite value.

This patch moves it into HTMLEditor and names it with same rule as
NodeIsBlockStatic().

Note that this method may return true if given node is unexpected node type.
E.g., comment node, CDATA node, etc. However, it's not scope of this bug.

Comment 4

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

Attachment: Bug 1574852 - part 3: Get rid of `HTMLEditRules::InitFields()`

It's called immediately before setting mHTMLEditor and sets mHTMLEditor to
nullptr. So, it does nothing actually. We can get rid of it.

Comment 5

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

Attachment: Bug 1574852 - part 4: Move `HTMLEditRules::GetInlineStyles()` to `HTMLEditor`

Comment 6

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

Attachment: Bug 1574852 - part 5: Move `HTMLEditRules::ReapplyCachedStyles()` to `HTMLEditor`

Comment 7

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

Attachment: Bug 1574852 - part 6: Move `HTMLEditRules::CacheInlineStyles()` to `HTMLEditor`

Comment 8

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

Attachment: Bug 1574852 - part 7: Get rid of `HTMLEditRules::ClearCachedStyles()`

It's declared, but not defined.

Comment 9

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

Attachment: Bug 1574852 - part 8: Move `HTMLEditRules::WillInsert()` to `HTMLEditor`

Comment 10

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

Attachment: Bug 1574852 - part 9: Move `HTMLEditRules::CreateStyleForInsertText()` to `HTMLEditor`

Comment 11

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

Attachment: Bug 1574852 - part 10: Move `HTMLEditRules::WillInsertText()` to `HTMLEditor`

Comment 12

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

Attachment: Bug 1574852 - part 11: Move `HTMLEditRules::GetTopEnclosingMailCite()` to `HTMLEditor`

Comment 13

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

Attachment: Bug 1574852 - part 12: Move `HTMLEditRules::SplitMailCites()` to `HTMLEditor`

Comment 14

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

Attachment: Bug 1574852 - part 13: Move `HTMLEditRules::CanContainParagraph()` to `HTMLEditor`

Comment 15

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

Attachment: Bug 1574852 - part 14: Move `HTMLEditRules::InsertBRElement()` to `HTMLEditor`

Comment 16

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

Attachment: Bug 1574852 - part 15: Move `HTMLEditUtils::GetHighestInlineParent()` to `HTMLEditor`

Comment 17

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

Attachment: Bug 1574852 - part 16: Move `HTMLEditRules::BustUpInlinesAtRangeEndpoints()` to `HTMLEditor`

Comment 18

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

Attachment: Bug 1574852 - part 17: Move `HTMLEditRules::GetInnerContent()` to `HTMLEditor`

Comment 19

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

Attachment: Bug 1574852 - part 18: Move `HTMLEditRules::BustUpInlinesAtBRs()` to `HTMLEditor`

Comment 20

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

Attachment: Bug 1574852 - part 19: Move `HTMLEditRules::GetNodesForOperation()` to `HTMLEditor`

This does not move the method simply. Instead, splits it to 4 simpler
methods.

Despite of the name, it modifies the DOM tree if aTouchContent is
TouchContent::yes. For avoiding this confusion and avoiding unnecessary
MOZ_CAN_RUN_SCRIPT attribute, the main part is split to
HTMLEditor::CollectEditTargetNodes().

If callers want to call HTMLEditRules::GetNodesForOperation() with
TouchContent::no, only calling this method equals to the call of
GetNodesForOperation().

Otherwise, the callers should call
HTMLEditor::SplitContentsAndCollectEditTargetNodes(). This calls internal
methods automatically.

First, HTMLEditor::SplitTextNodesAtRangeEnd() splits text nodes at end of
each range. Then, HTMLEditor::SplitParentInlineElementsAtRangeEdges()
overload splits inline elements at both edges of each range. Then, collects
event target nodes with calling HTMLEditor::CollectEditTargetNodes().
Finally, HTMLEditor::MaybeSplitElementsAtEveryBRElement() may split the
result nodes at every <br> element in them.

Comment 21

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

Attachment: Bug 1574852 - part 20: Move `HTMLEditRules::GetPromotedPoint()` to `HTMLEditor`

HTMLEditRules::GetPromotedPoint() does too many things. Therefore, this patch
splits it to 3 methods. One is for specific EditSubAction values, that's
the first block in GetPromotedPoint(). It's named as
GetWhiteSpaceEndPoint(). Next one is for expanding start of the range to
start of its line. It's named as GetCurrentPhysicalLineStartPoint(). The
last one is for expanding end of the range to end of its line. It's named as
GetCurrentPhysicalLineEndPoint().

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ae319789645
part 1: Get rid of `HTMLEditRules::IsBlockNode()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/72a225ec2ee2
part 2: Replace `HTMLEditRules::IsInlineNode()` with `HTMLEditor::NodeIsInlineStatic()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c51084cb4fb0
part 3: Get rid of `HTMLEditRules::InitFields()` r=m_kato

Comment 23

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/2ae319789645
https://hg.mozilla.org/mozilla-central/rev/72a225ec2ee2
https://hg.mozilla.org/mozilla-central/rev/c51084cb4fb0

Comment 24

[Pulsebot]

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

Comment 25

[Pulsebot]

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

Comment 26

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

Attachment: Bug 1574852 - part 21: Move `HTMLEditRules::PromoteRange()` to `HTMLEditor`

The method name is really unclear what's done. The method does 3 things.
One is try to select a <br> element in empty block if given range is
collapsed in the block. This is moved as
HTMLEditor::SelectBRElementIfCollapsedInEmptyBlock(). Next, it extends the
given range to include adjuscent whitespaces only when edit sub-action is
inserting or deleting text. This is moved as
HTMLEditor::CreateRangeIncludingAdjuscentWhiteSpaces(). Finally, when
handling the other edit sub-actions, extends the given range to start/end
of line at both edges. This is moved as
HTMLEditor::CreateRangeExtendedToHardLineStartAndEnd().

And also this patch makes each caller of PromoteRange() to check edit
sub-action by themselves. Unfortunately, this duplicates same logic to
multiple places, but makes what they do clearer. I think that the duplication
issue should be fixed later if necessary. Instead, we should shine the
blackbox of HTMLEditRules right now.

Comment 27

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

Attachment: Bug 1574852 - part 22: Move `HTMLEditRules::GetPromotedRanges()` to `HTMLEditor`

HTMLEditRules::GetPromotedRanges() does 2 things. Calling
CreateRangeIncludingAdjuscentWhiteSpaces() or
CreateRangeExtendedToHardLineStartAndEnd() with each range of Selection.
The difference is considered with current edit sub-action. Therefore, we cal
split it to GetSelectionRangesExtendedToIncludeAdjuscentWhiteSpaces() and
GetSelectionRangesExtendedToHardLineStartAndEnd(). Then, we got clearer code
at each caller.

Comment 28

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

Attachment: Bug 1574852 - part 23: Move `HTMLEditRules::GetNodesFromSelection()` to `HTMLEditor`

HTMLEditRules::GetNodesFromSelection() has a patch to call
HTMLEditor::GetSelectionRangesExtendedToIncludeAdjuscentWhiteSpaces().
However, nobody uses this path so that we can get rid of this path.
Then, it becomes just calling SplitInlinesAndCollectEditTargetNodes() or
CollectEditTargetNodes() with result of
GetSelectionRangesExtendedToHardLineStartAndEnd(). Therefore, we can split
it to SplitInlinesAndCollectEditTargetNodesInExtendedSelectionRanges() and
CollectEditTargetNodesInExtendedSelectionRanges(). Then, we can mark
only the former as MOZ_CAN_RUN_SCRIPT.

Comment 29

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

Attachment: Bug 1574852 - part 24: Move `HTMLEditRules::GetNodesFromPoint()` to `HTMLEditor`

It's called only from HTMLEditRules::MoveBlock(). Even though it has 4
patters to call different methods, we need only one of them. Therefore,
this patch moves it into HTMLEditor.h as
SplitInlinesAndCollectEditTargetNodesInOneHardLine().

Comment 30

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

Attachment: Bug 1574852 - part 25: Move `HTMLEditRules::GetChildNodesForOperation()` to `HTMLEditor`

It just collects all children of given node so that it can be a static method
in HTMLEditor.

Comment 31

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

Attachment: Bug 1574852 - part 26: Move a part of `HTMLEditRules::LookInsideDivBQandList()` and remove the others

HTMLEditRules::LookInsideDivBQandList() does complicated things and I cannot
explain with a method name what it does. Fortunately, there are only 2 callers.
Therefore, this patch duplicates the part of modifying the range and creates
a method to retrieve "deepest and only editable child of <div>, <blockquote>
and one of list items".

Comment 32

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

Attachment: Bug 1574852 - part 27: Move first half of `HTMLEditRules::GetListActionNodes()` to `HTMLEditor` and each caller

First half of HTMLEditoRules::GetListActionNodes() does 2 things. One is
trying to get parent list element of Selection ranges if aEntireList is
EntireList::yes. If it found a list element, it does nothing anymore.
Otherwise, falls backs to EntireList::no case. So, if each caller which
calls it with EntireList::yes, GetListActionNodes() does not need the
argument. Therefore, this patch does it first.

Then, GetListActionNodes() calls
CollectEditTargetNodesInExtendedSelectionRanges() or
SplitInlinesAndCollectEditTargetNodesInExtendedSelectionRanges(). It's
considered with aTouchContent is yes or no. So, this should be done
by each caller for making it clearer.

Comment 33

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

Attachment: Bug 1574852 - part 28: Make methods collecting event target nodes take additional argument which can specify whether `aOutArrayOfNodes` includes non-editable nodes or not

HTMLEditRules::GetListActionNodes() removes non-editable element. However,
this should've been done by collector methods.

Comment 34

[Noemi Erli[:noemi_erli]]

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&revision=6456b7a6cc679d744637e639d37cb73e795c1357&searchStr=os%2Cx%2C10.14%2Cshippable%2Copt%2Cmochitests%2Ctest-macosx1014-64-shippable%2Fopt-mochitest-devtools-chrome-e10s-1%2Cm%28dt1%29

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=262875579&repo=autoland&lineNumber=3044

Backout: https://hg.mozilla.org/integration/autoland/rev/2ce3d67170d9c443f2e856460d0fe95a151a3509

Comment 35

[Pulsebot]

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

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cc701cc6957
part 6: Move `HTMLEditRules::CacheInlineStyles()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/4c591d763526
part 7: Get rid of `HTMLEditRules::ClearCachedStyles()` r=m_kato

Comment 37

[Pulsebot]

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

Comment 38

[Pulsebot]

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

Comment 39

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/bfbdc9ba712c
https://hg.mozilla.org/mozilla-central/rev/9a87c2e0fea5

Comment 40

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/7cc701cc6957
https://hg.mozilla.org/mozilla-central/rev/4c591d763526
https://hg.mozilla.org/mozilla-central/rev/43bfa576a15e
https://hg.mozilla.org/mozilla-central/rev/da6c34e227b2

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12ec2813293d
part 10: Move `HTMLEditRules::WillInsertText()` to `HTMLEditor` r=m_kato

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ab23a7388e2
part 11: Move `HTMLEditRules::GetTopEnclosingMailCite()` to `HTMLEditor` r=m_kato

Comment 43

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

Attachment: Bug 1574852 - part 29: Merge `HTMLEditRules::GetListActionNodes()` with `HTMLEditor::CollectEditTargetNodes()`

This patch fixes an existing bug with this clean up.

Except HTMLEditRules::MoveBlock(), GetListActionNodes() is called after
calling SplitInlinesAndCollectEditTargetNodesInExtendedSelectionRanges()
or CollectEditTargetNodesInExtendedSelectionRanges() with
EditSubAction::eCreateOrChangeList. I think that HTMLEditRules::MoveBlock()
using the edit sub-action is a simple mistake. Perhaps, it should be
EditSubAction::eCreateOrRemvoeBlock. However, I'm not 100% sure because
HTMLEditor::CollectEditTargetNodes() does special handling for
EditSubAction::eCreateOrRemvoeBlock but not so for
EditSubAction::eCreateOrChangeList. The behavior of difference between
those edit sub-actions are only here. Therefore, this patch creates new
EditSubAction eMergeBlockContents for MoveBlock().

Then, this patch makes HTMLEditor::CollectEditTargetNodes() handle
EditSubAction::eCreateOrChangeList insead of GetListActionNodes().
This causes one logic change in SplitInlinesAndCollectEditTargetNodes().
It calls MaybeSplitElementsAtEveryBRElement() after CollectEditTargetNodes()
so that this change makes calling MaybeSplitElementsAtEveryBRElement() at
last. According to my tests, new behavior must be expected since <br>
elements outside and in <table> should be handled consistently. Therefore,
this patch adds some simple testcases into WPT.

Comment 44

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

Attachment: Bug 1574852 - part 30: Move `HTMLEditRules::NormalizeSelection()` to `HTMLEditor`

Comment 45

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

Attachment: Bug 1574852 - part 31: Move `HTMLEditRules::IsEmptyInline()` and `HTMLEditRules::ListIsEmptyLine()` to `HTMLEditor`

Comment 46

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

Attachment: Bug 1574852 - part 32: Move `HTMLEditRules::MaybeSplitAncestorsForInsertWithTransaction()` to `HTMLEditor`

Comment 47

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

Attachment: Bug 1574852 - part 33: Move `HTMLEditRules::MakeBlockquote()` to `HTMLEditor`

Comment 48

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

Attachment: Bug 1574852 - part 34: Move `HTMLEditRules::SplitRangeOffFromBlock()` to `HTMLEditor`

Comment 49

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

Attachment: Bug 1574852 - part 35: Move `HTMLEditRules::SplitRangeOffFromBlockAndRemoveMiddleContainer()` to `HTMLEditor`

Comment 50

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

Attachment: Bug 1574852 - part 36: Move `HTMLEditRules::RemvoeBlockStyle()` to `HTMLEditor`

Comment 51

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

Attachment: Bug 1574852 - part 37: Move `HTMLEditRules::ApplyBlockStyle()` to `HTMLEditor`

Comment 52

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

Attachment: Bug 1574852 - part 38: Move `HTMLEditRules::MakeBasicBlock()` to `HTMLEditor`

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41bb079385e1
part 12: Move `HTMLEditRules::SplitMailCites()` to `HTMLEditor` r=m_kato

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/236e278e33a4
part 13: Move `HTMLEditRules::CanContainParagraph()` to `HTMLEditor` r=m_kato

Comment 55

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f41a0d889470
part 14: Move `HTMLEditRules::InsertBRElement()` to `HTMLEditor` r=m_kato

Comment 56

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a2413d91a19
part 15: Move `HTMLEditUtils::GetHighestInlineParent()` to `HTMLEditor` r=m_kato

Comment 57

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/12ec2813293d
https://hg.mozilla.org/mozilla-central/rev/4ab23a7388e2
https://hg.mozilla.org/mozilla-central/rev/41bb079385e1

Comment 58

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6174156c2fff
part 16: Move `HTMLEditRules::BustUpInlinesAtRangeEndpoints()` to `HTMLEditor` r=m_kato

Comment 59

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa5c785f9005
part 17: Move `HTMLEditRules::GetInnerContent()` to `HTMLEditor` r=m_kato

Comment 60

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3dc91a89d99
part 18: Move `HTMLEditRules::BustUpInlinesAtBRs()` to `HTMLEditor` r=m_kato

Comment 61

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/166c445265ff
part 19: Move `HTMLEditRules::GetNodesForOperation()` to `HTMLEditor` r=m_kato

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c980a90fce4f
part 20: Move `HTMLEditRules::GetPromotedPoint()` to `HTMLEditor` r=m_kato

Comment 63

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/236e278e33a4
https://hg.mozilla.org/mozilla-central/rev/f41a0d889470
https://hg.mozilla.org/mozilla-central/rev/3a2413d91a19
https://hg.mozilla.org/mozilla-central/rev/6174156c2fff

Comment 64

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/fa5c785f9005
https://hg.mozilla.org/mozilla-central/rev/b3dc91a89d99
https://hg.mozilla.org/mozilla-central/rev/166c445265ff
https://hg.mozilla.org/mozilla-central/rev/c980a90fce4f

Comment 65

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd856fd9fc39
part 21: Move `HTMLEditRules::PromoteRange()` to `HTMLEditor` r=m_kato

Comment 66

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c5adf2e84f97
part 22: Move `HTMLEditRules::GetPromotedRanges()` to `HTMLEditor` r=m_kato

Comment 67

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19803f3decdd
part 23: Move `HTMLEditRules::GetNodesFromSelection()` to `HTMLEditor` r=m_kato

Comment 68

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/cd856fd9fc39
https://hg.mozilla.org/mozilla-central/rev/c5adf2e84f97
https://hg.mozilla.org/mozilla-central/rev/19803f3decdd

Comment 69

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1df6caf3b1d
part 24: Move `HTMLEditRules::GetNodesFromPoint()` to `HTMLEditor` r=m_kato

Comment 70

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/d1df6caf3b1d

Comment 71

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2dfef8511618
part 25: Move `HTMLEditRules::GetChildNodesForOperation()` to `HTMLEditor` r=m_kato

Comment 72

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/2dfef8511618

Comment 73

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74ce25be0e9a
part 26: Move a part of `HTMLEditRules::LookInsideDivBQandList()` and remove the others r=m_kato

Comment 74

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ada62f7a9760
part 27: Move first half of `HTMLEditRules::GetListActionNodes()` to `HTMLEditor` and each caller r=m_kato

Comment 75

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65b324e80076
part 28: Make methods collecting event target nodes take additional argument which can specify whether `aOutArrayOfNodes` includes non-editable nodes or not r=m_kato

Comment 76

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65a9f716e652
part 29: Merge `HTMLEditRules::GetListActionNodes()` with `HTMLEditor::CollectEditTargetNodes()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8804f2b110be
part 30: Move `HTMLEditRules::NormalizeSelection()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c3e4c8a6b1ff
part 31: Move `HTMLEditRules::IsEmptyInline()` and `HTMLEditRules::ListIsEmptyLine()` to `HTMLEditor` r=m_kato

Comment 77

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/18659 for changes under testing/web-platform/tests

Comment 78

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 79

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/74ce25be0e9a
https://hg.mozilla.org/mozilla-central/rev/ada62f7a9760
https://hg.mozilla.org/mozilla-central/rev/65b324e80076
https://hg.mozilla.org/mozilla-central/rev/65a9f716e652
https://hg.mozilla.org/mozilla-central/rev/8804f2b110be
https://hg.mozilla.org/mozilla-central/rev/c3e4c8a6b1ff

Comment 80

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 81

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15bfd420d024
part 32: Move `HTMLEditRules::MaybeSplitAncestorsForInsertWithTransaction()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8257421bca60
part 33: Move `HTMLEditRules::MakeBlockquote()` to `HTMLEditor` r=m_kato

Comment 82

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c701cf449d2c
part 34: Move `HTMLEditRules::SplitRangeOffFromBlock()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/9d63a239b705
part 35: Move `HTMLEditRules::SplitRangeOffFromBlockAndRemoveMiddleContainer()` to `HTMLEditor` r=m_kato

Comment 83

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85b7e9527466
part 36: Move `HTMLEditRules::RemvoeBlockStyle()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/2871d02f3f7b
part 37: Move `HTMLEditRules::ApplyBlockStyle()` to `HTMLEditor` r=m_kato

Comment 84

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/15bfd420d024
https://hg.mozilla.org/mozilla-central/rev/8257421bca60
https://hg.mozilla.org/mozilla-central/rev/c701cf449d2c
https://hg.mozilla.org/mozilla-central/rev/9d63a239b705
https://hg.mozilla.org/mozilla-central/rev/85b7e9527466
https://hg.mozilla.org/mozilla-central/rev/2871d02f3f7b

Comment 85

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9b024555ac1
part 38: Move `HTMLEditRules::MakeBasicBlock()` to `HTMLEditor` r=m_kato

Comment 86

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/d9b024555ac1

Comment 87

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

Attachment: Bug 1574852 - part 39: Move `HTMLEditRules::InsertBRIfNeeded()` to `HTMLEditor`

Comment 88

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

Attachment: Bug 1574852 - part 40: Move `HTMLEditRules::InsertPaddingBRElementForEmptyLastLineIfNeeded()` to `HTMLEditor`

And this fixes the caller which has not guaranteed the lifetime of the
start container.

Comment 89

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

Attachment: Bug 1574852 - part 41: Move `HTMLEditRules::InsertBRIfNeeded(nsINode&)` to `HTMLEditor`

Comment 90

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

Attachment: Bug 1574852 - part 42: Merge `HTMLEditRules::WillMakeBasicBlock()` into `HTMLEditor::InsertBasicBlockWithTransaction()`

HTMLEditRules::WillMakeBasicBlock() just calls
HTMLEditor::FormatBlockContainer() and it's called only for
EditSubAction::eCreateOrRemoveBlock and it's used only in
HTMLEditor::InsertBasicBlockWithTransaction(). Therefore, we can replace
calling HTMLEditRules::WillDoAction() in it with what
HTMLEditRules::WIllMakeBasicBlock() does.

First, HTMLEditRules::WillDoAction() checks whether first selection range
is editable. If it's not so, it returns NS_OK with setting aCancel to
true. Therefore, this patch moves this part to
HTMLEditor::CanHandleHTMLEditSubAction() for making
HTMLEditor::InsertBasicBlockWithTransaction() can check it easier.

Next, HTMLEditor::InsertBasicBlockWithTransaction() does something if
HTMLEditRules::WillDoAction() returns as not handled nor canceled.
However, this special handling requires at least one selection range:
https://searchfox.org/mozilla-central/rev/597a69c70a5cce6f42f159eb54ad1ef6745f5432/editor/libeditor/HTMLEditor.cpp#2284-2288
Surprisingly, handled is set to false only when there is an error or
there is no selection range. Therefore, this long block has already been
dead code so that we can remove this. Removing this block causes that we
become not throwing exception when Document.execCommand("formatblock")
without selection ranges. But this is better since Chrome does not throw
excption.

Finally, this patch renames some related methods:

• HTMLEditor::FormatBlockContainer() -> HTMLEditor::FormatBlockContainerWithTransaction()
• HTMLEditor::InsertBasicBlockWithTransaction() -> HTMLEditor::FormatBlockContainerAsSubAction()

Comment 91

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

Attachment: Bug 1574852 - part 43: Move `HTMLEditRules::DidMakeBasicBlock()` to `HTMLEditor`

Comment 92

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

Attachment: Bug 1574852 - part 44: Move `HTMLEditRules::IsEmptyBlockElement()` to `HTMLEditor`

Comment 93

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

Attachment: Bug 1574852 - part 45: Move `HTMLEditRules::DefaultParagraphSeparator()` to `HTMLEditor`

Comment 94

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

Attachment: Bug 1574852 - part 46: Move `HTMLEditRules::SplitParagraph()` to `HTMLEditor`

Comment 95

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

Attachment: Bug 1574852 - part 47: Move `HTMLEditRules::ReturnInParagraph()` to `HTMLEditor`

Comment 96

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

Attachment: Bug 1574852 - part 48: Move `HTMLEditRules::ReturnInHeader()` to `HTMLEditor`

Comment 97

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

Attachment: Bug 1574852 - part 49: Move `HTMLEditRules::ReturnInListItem()` to `HTMLEditor`

Comment 98

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

Attachment: Bug 1574852 - part 50: Move `TextEditRules::UndefineCaretBidiLevel()` to `EditorBase`

It's used both by TextEditRules and HTMLEditRules so that EditorBase
should have it instead.

Comment 99

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

Attachment: Bug 1574852 - part 51: Move `HTMLEditRules::IsInListItem()` to `HTMLEditor`

Comment 100

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

Attachment: Bug 1574852 - part 52: Make `HTMLEditRules::WillInsertParagraph()` merged with `HTMLEditor::InsertParagraphSeparatorAsSubAction()`

Meaningful job of HTMLEditor::InsertParagraphSeparatorAsSubAction() is only
calling HTMLEditRules::WillInsertParagraph() via
HTMLEditRules::WillDoAction(). Therefore, we can move all jobs in them
into HTMLEditRules::WillInsertParagraph() and rename it to
HTMLEditor::InsertParagraphSeparatorAsSubAction().

Comment 101

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

Attachment: Bug 1574852 - part 53: Move `HTMLEditRuies::InDifferentTableElements()` to `HTMLEditor`

Comment 102

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

Attachment: Bug 1574852 - part 54: Move `HTMLEditRules::ConvertListType()` to `HTMLEditor`

Comment 103

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

Attachment: Bug 1574852 - part 55: Move `HTMLEditRules::MakeList()` to `HTMLEditor`

Additionally, this patch makes it use early-return style with continue as
far as making number of changing line minimized.

Comment 104

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

Attachment: Bug 1574852 - part 56: Merge `HTMLEditRules::WillMakeList()` and `HTMLEditRules::WillMakeDefinitionList()` and make `HTMLEditor` call it directly

HTMLEditRules::WillMakeDefinitionList() just calls
HTMLEditRules::WillMakeList() and HTMLEditRules::WillMakeList() can be
called as HTMLEditRules::MakeOrChangeListAndListItemAsSubAction() so that
we should merge them and make HTMLEditor call it directly.

This patch also removes default action part of
HTMLEditor::MakeOrChangeListAsAction() because it runs only when
HTMLEditRules::WillDoAction() does not return canceled nor error but not
handled, however, it's won't occur since HTMLEditRules::WillMakeList()
always sets aHandled to true when it returns NS_OK.

Comment 105

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

Attachment: Bug 1574852 - part 57: Move `HTMLEditRules::DeleteNodeIfCollapsedText()` to `HTMLEditor`

Comment 106

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

Attachment: Bug 1574852 - part 58: Move `HTMLEditRules::CheckForInvisibleBR()` to `HTMLEditor`

This is defined as too complicated than what it does actually since there
was not EditorDOMPointBase.

If given point's offset is 0, returns nullptr. If aWhere is
BRLocation::blockEnd, treats aOffset is length of aBlock. Otherwise,
using given point. Then, if the WSRun start reason is br, returns the
start reason node. This means that this method just retrieves invisible
<br> element if it starts WSRun at given point.

Now, callers can specify end of block with EditorDOMPointBase::SetToEndOf()
easier. Therefore, we can make it take only an EditorDOMPointBase instance.

Comment 107

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

Attachment: Bug 1574852 - part 59: Move `HTMLEditRules::JoinNearestEditableNodesWithTransaction()` to `HTMLEditor`

Comment 108

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

Attachment: Bug 1574852 - part 60: Move `HTMLEditRules::MoveNodeSmart()` and `HTMLEditRules::MoveContents()` to `HTMLEditor`

Making them return next insertion point with nsresult makes the callers
easier to understand. Therefore, this patch declares MoveNodeResult class
newly and make them use it.

And also we can get rid of the case of setting *aInOutDestOffset to -1
because it's a hacky case of HTMLEditRules::MoveBlock(). If we keep it,
MoveNodeSmart() needs to compute new insertion point with different logic.
Therefore, this patch makes HTMLEditRules::MoveBlock() adjust new insertion
point with checking its argument.

Comment 109

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

Attachment: Bug 1574852 - part 61: Move `HTMLEditRules::MoveBlock()` to `HTMLEditor`

Comment 110

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

Attachment: Bug 1574852 - part 62: Move `HTMLEditRules::TryToJoinBlocksWithTransaction()` to `HTMLEditor`

Additionally, WSRunObject::ScrabBlockBoundary() and
WSRunObject::PreparetToJoinBlocks() are used only the it. Therefore, we
can clean them up.

Comment 111

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

Attachment: Bug 1574852 - part 63: Move `HTMLEditRules::GetGoodSelPointForNode()` to `HTMLEditor`

Comment 112

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

Attachment: Bug 1574852 - part 64: Move `HTMLEditRules::MaybeDeleteTopMostEmptyAncestor()` to `HTMLEditor`

Comment 113

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

Attachment: Bug 1574852 - part 65: Move `TextEditRules::CheckBidiLevelForDeletion()` to `EditorBase`

Comment 114

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

Attachment: Bug 1574852 - part 66: Move `HTMLEditRules::ExpandSelectionForDelete()` to `HTMLEditor`

And this patch makes the new method do not touch Selection, instead, return
new StaticRange. Although the creation cost may make damage to the
performance but let's keep using StaticRange for now. There should be
a stack only class which have 2 RangeBoundaryBase or EditorDOMPointBase.

Comment 115

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4e7a78eb9a70
part 39: Move `HTMLEditRules::InsertBRIfNeeded()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a67e86b1c73f
part 40: Move `HTMLEditRules::InsertPaddingBRElementForEmptyLastLineIfNeeded()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/21a5dd1e0ad3
part 41: Move `HTMLEditRules::InsertBRIfNeeded(nsINode&)` to `HTMLEditor` r=m_kato

Comment 116

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12607b62d742
part 42: Merge `HTMLEditRules::WillMakeBasicBlock()` into `HTMLEditor::InsertBasicBlockWithTransaction()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c2747317b63f
part 43: Move `HTMLEditRules::DidMakeBasicBlock()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/aaff83765792
part 44: Move `HTMLEditRules::IsEmptyBlockElement()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/d1454eea0488
part 45: Move `HTMLEditRules::DefaultParagraphSeparator()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/e3330dd0a807
part 46: Move `HTMLEditRules::SplitParagraph()` to `HTMLEditor` r=m_kato

Comment 117

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/4e7a78eb9a70
https://hg.mozilla.org/mozilla-central/rev/a67e86b1c73f
https://hg.mozilla.org/mozilla-central/rev/21a5dd1e0ad3

Comment 118

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

Attachment: Bug 1574852 - part 67-1: Make `HTMLEditRules::WillDeleteSelection() return `EditActionResult`

Before moving the largest method in our tree, we should split it. This is
first preparation of that.

Comment 119

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

Attachment: Bug 1574852 - part 67-2: Rename variables in `HTMLEditRules::WillDeleteSelection()`

Comment 120

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

Attachment: Bug 1574852 - part 67-3: Split `HTMLEditRules::WillDeleteSelection()` to 3 methods

First, we can split it with 3 parts:

1. preparation with current selection part.
2. handling deletion with collapsed selection part.
3. handling deletion with non-collapsed selection part.
   The first should leave in WillDeleteSelection(), but the others should be
   moved to independent methods.

With this change, we need to fix an odd case when there is no visible node
and GetFirstSelectedTableCellElement() returned error due to crossing
the method boundary. The method returns error only when there is no selection
ranges. In such case, we cannot handle deletion of current selection anyway.
So, it must not be a problem to handle the error immediately after calling
GetFirstSelectedTableCellElement(). Note that this shouldn't occur in
normal cases since WillDoAction() checks it before calling
WillDeleteSelection().

Comment 121

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

Attachment: Bug 1574852 - part 67-4: Split `HTMLEditRules::HandleDeleteAroundCollapsedSelection()`

Comment 122

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

Attachment: Bug 1574852 - part 67-5: Rewrite `HTMLEditRules::HandleDeleteNonCollapsedSelection()` with `EditorDOMPoint`

Before splitting the method, we should make it use EditorDOMPoint to
treat selection edges.

Comment 123

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

Attachment: Bug 1574852 - part 67-6: Split the last common part of `HTMLEditRules::HandleDeleteNonCollapsedSelection()`

Comment 124

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

Attachment: Bug 1574852 - part 67-7: Make early-return style for first simple deletion case of `HTMLEditRules::HandleDeleteNonCollapsedSelection()`

Comment 125

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

Attachment: Bug 1574852 - part 67-8: Make next simple deletion code use early-return style

Comment 126

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

Attachment: Bug 1574852 - part 67-9: Make next early return-style code work without `AutoTrackDOMPoint`

The next simple case is using early-return style and does not refer modified
firstRangeStart nor firstRangeEnd so that it can do same thing without
AutoTrackDOMPoint.

Then, there is remaining the complicated case. We cannot split it anymore.

Note that we should not split HandleDeleteNonCollapsedSelection() anymore
because as you know, it does not make sense. It checks only first range
and consider how to treat all ranges. We should split it after we investigate
deeper with writing new WPT.

Comment 127

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

Attachment: Bug 1574852 - part 67-10: Move `HTMLEditRules::WillDeleteSelection()` and related methods to `HTMLEditor`

Comment 128

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

Attachment: Bug 1574852 - part 68: Make `HTMLEditRules::GetDefinitionListItemTypes()` to a stack class

It scans children and returns whether <dt> and <dd> are found or not.
So, we can make it a stack class and makes caller pick the necessary value
with getter methods.

Comment 129

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

Attachment: Bug 1574852 - part 69: Move `HTMLEditRules::PopListItem()` to `HTMLEditor`

Comment 130

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

Attachment: Bug 1574852 - part 70: Move `HTMLEditRules::RemoveListStructur()` to `HTMLEditor`

Comment 131

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

Attachment: Bug 1574852 - part 71: Move `HTMLEditRules::WillRemoveList()` to `HTMLEditor`

Comment 132

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

Attachment: Bug 1574852 - part 72: Move `HTMLEditRules::ChangeMarginStart()` to `HTMLEditor` and get rid of its wrappers

Comment 133

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/12607b62d742
https://hg.mozilla.org/mozilla-central/rev/c2747317b63f
https://hg.mozilla.org/mozilla-central/rev/aaff83765792
https://hg.mozilla.org/mozilla-central/rev/d1454eea0488
https://hg.mozilla.org/mozilla-central/rev/e3330dd0a807

Comment 134

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99ec3f15612f
part 47: Move `HTMLEditRules::ReturnInParagraph()` to `HTMLEditor` r=m_kato

Comment 135

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15b069730cb6
part 48: Move `HTMLEditRules::ReturnInHeader()` to `HTMLEditor` r=m_kato

Comment 136

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/99ec3f15612f

Comment 137

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/428adee67846
part 49: Move `HTMLEditRules::ReturnInListItem()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/d00ed80a7115
part 50: Move `TextEditRules::UndefineCaretBidiLevel()` to `EditorBase` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/fc87b392688b
part 51: Move `HTMLEditRules::IsInListItem()` to `HTMLEditor` r=m_kato

Comment 138

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ece9cd0da19
part 52: Make `HTMLEditRules::WillInsertParagraph()` merged with `HTMLEditor::InsertParagraphSeparatorAsSubAction()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/b7d720cd37f9
part 53: Move `HTMLEditRuies::InDifferentTableElements()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/fae7917900bd
part 54: Move `HTMLEditRules::ConvertListType()` to `HTMLEditor` r=m_kato

Comment 139

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/15b069730cb6

Comment 140

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0c0302b3db46
part 55: Move `HTMLEditRules::MakeList()` to `HTMLEditor` r=m_kato

Comment 141

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/428adee67846
https://hg.mozilla.org/mozilla-central/rev/d00ed80a7115
https://hg.mozilla.org/mozilla-central/rev/fc87b392688b
https://hg.mozilla.org/mozilla-central/rev/7ece9cd0da19
https://hg.mozilla.org/mozilla-central/rev/b7d720cd37f9
https://hg.mozilla.org/mozilla-central/rev/fae7917900bd
https://hg.mozilla.org/mozilla-central/rev/0c0302b3db46

Comment 142

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b08d2e18630f
part 56: Merge `HTMLEditRules::WillMakeList()` and `HTMLEditRules::WillMakeDefinitionList()` and make `HTMLEditor` call it directly r=m_kato
https://hg.mozilla.org/integration/autoland/rev/38bf4be88279
part 57: Move `HTMLEditRules::DeleteNodeIfCollapsedText()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/576017f68d3a
part 58: Move `HTMLEditRules::CheckForInvisibleBR()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/363362abb3e9
part 59: Move `HTMLEditRules::JoinNearestEditableNodesWithTransaction()` to `HTMLEditor` r=m_kato

Comment 143

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/082159fea413
part 60: Move `HTMLEditRules::MoveNodeSmart()` and `HTMLEditRules::MoveContents()` to `HTMLEditor` r=m_kato

Comment 144

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd2b83d5dda7
part 61: Move `HTMLEditRules::MoveBlock()` to `HTMLEditor` r=m_kato

Comment 145

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

Attachment: Bug 1574852 - part 73: Move `HTMLEditRules::IndentAroundSelectionWithCSS()` to `HTMLEditor`

And also this patch fixes unexpected behavior change by bug 1460509:
https://searchfox.org/mozilla-central/diff/d5d67de86f23655fcccc7bbcf4423bb75148fd34/editor/libeditor/HTMLEditRules.cpp#4466

Comment 146

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

Attachment: Bug 1574852 - part 74: Move `HTMLEditRules::IndentAroundSelectionWithHTML()` to `HTMLEditor`

Comment 147

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

Attachment: Bug 1574852 - part 75: Move `HTMLEditRules::OutdentPartOfBlock()` to `HTMLEditor`

Comment 148

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

Attachment: Bug 1574852 - part 76: Move `HTMLEditRules::OutdentAroundSelection()` to `HTMLEditor`

Comment 149

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

Attachment: Bug 1574852 - part 77: Move `HTMLEditRules::WillIndent()`, `HTMLEditRules::WillHTMLIndent()` and `HTMLEditRules::WillCSSIndent()` to `HTMLEditor`

The caller of HTMLEditRules::WillDoAction() is shared with "outdent".
Therefore, this patch is a preparation of makes HTMLEditor stop calling it.

Be aware, HTMLEditRules::WillIndent() won't cancel the action, and also
always handles the action unless editor has already been destroyed. Therefore,
we can remove the dead code in HTMLEditor::IndentOrOutdentAsSubAction()
right now.

Comment 150

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

Attachment: Bug 1574852 - part 78: Move `HTMLEditRules::WillOutdent()` to `HTMLEditor`

Comment 151

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

Attachment: Bug 1574852 - part 79: Make `HTMLEditor::IndentAsAction()` and `HTMLEditor::OutdentAsAction()` call specific `AsSubAction()` methods instead of `WillDoAction()` and `DidDoAction()`

Comment 152

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

Attachment: Bug 1574852 - part 80: Move `HTMLEditRules::AlignBlockContents()` to `HTMLEditor`

Comment 153

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

Attachment: Bug 1574852 - part 81: Move `HTMLEditRules::AlignInnerBlocks()` to `HTMLEditor`

Comment 154

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

Attachment: Bug 1574852 - part 82: Move `HTMLEditRules::MakeTransitionList()` to `HTMLEditor`

Comment 155

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

Attachment: Bug 1574852 - part 83: Move `HTMLEditRules::MakeSureElemStartsOrEndsOnCR()` to `HTMLEditor`

HTMLEditRules::MakeSureElemStartsOrEndsOnCR() is unclear what it does.
And fortunately, its job is simple if we split it to each mode. Therefore,
this patch splits it to EnsureHardLineBeginsWithFirstChildOf() and
EnsureHardLineEndsWithLastChildOf(). Finally, the batch caller of them,
HTMLEditRules::MakeSureElemStartsAndEndsOnCR() is removed since directly
calling both of them is clearer.

Comment 156

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

Attachment: Bug 1574852 - part 84: Move `HTMLEditRules::RemoveAlignment()` to `HTMLEditor`

And also this patch removes unnecessary declarations in HTMLEditRules.h.

Comment 157

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

Attachment: Bug 1574852 - part 85: Move `HTMLEditRules::AlignBlock()` to `HTMLEditor`

With guaranteeing the argument element type with MOZ_ASSERT(), we can
make it really simpler.

Comment 158

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

Attachment: Bug 1574852 - part 86: Move `HTMLEditRules::WillAlign()` and `HTMLEditRules::AlignContentsAtSelection()` to `HTMLEditor`

And also this splits large 2 blocks of
HTMLEditRules::AlignContentsAtSelection() to 2 methods.

Comment 159

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

Attachment: Bug 1574852 - part 87: Move `HTMLEditRules::SelectionEndpointInNode()` to `HTMLEditor`

Comment 160

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

Attachment: Bug 1574852 - part 88: Move `HTMLEditRules::FindNearEditableNode()` to `HTMLEditor`

Comment 161

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

Attachment: Bug 1574852 - part 89: Move `HTMLEditRules::AdjustSelection()` to `HTMLEditor`

Comment 162

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

Attachment: Bug 1574852 - part 90: Move `HTMLEditRules::ConfirmSelectionInBody()` to `HTMLEditor`

Comment 163

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

Attachment: Bug 1574852 - part 91: Move `HTMLEditRules::InsertBRElementToEmptyListItemsAndTableCellsInRange()` to `HTMLEditor`

Comment 164

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

Attachment: Bug 1574852 - part 92: Move `HTMLEditRules::RemoveEmptyNodesInChangedRange()` to `HTMLEditor`

Comment 165

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

Attachment: Bug 1574852 - part 93: Move `HTMLEditRules::CheckInterlinePosition()` to `HTMLEditor`

Comment 166

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

Attachment: Bug 1574852 - part 94: Move `HTMLEditRules::PinSelectionToNewBlock()` to `HTMLEditor`

Comment 167

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

Attachment: Bug 1574852 - part 95: Move `HTMLEditRules::AfterEditInner()` to `HTMLEditor`

Comment 168

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ced245d5835
part 62: Move `HTMLEditRules::TryToJoinBlocksWithTransaction()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/4b5edaf3b72c
part 63: Move `HTMLEditRules::GetGoodSelPointForNode()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/81c375145c55
part 64: Move `HTMLEditRules::MaybeDeleteTopMostEmptyAncestor()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/4d5dde4e72b4
part 65: Move `TextEditRules::CheckBidiLevelForDeletion()` to `EditorBase` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/68fad864eca4
part 66: Move `HTMLEditRules::ExpandSelectionForDelete()` to `HTMLEditor` r=m_kato

Comment 169

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/b08d2e18630f
https://hg.mozilla.org/mozilla-central/rev/38bf4be88279
https://hg.mozilla.org/mozilla-central/rev/576017f68d3a
https://hg.mozilla.org/mozilla-central/rev/363362abb3e9
https://hg.mozilla.org/mozilla-central/rev/082159fea413
https://hg.mozilla.org/mozilla-central/rev/cd2b83d5dda7
https://hg.mozilla.org/mozilla-central/rev/9ced245d5835
https://hg.mozilla.org/mozilla-central/rev/4b5edaf3b72c
https://hg.mozilla.org/mozilla-central/rev/81c375145c55
https://hg.mozilla.org/mozilla-central/rev/4d5dde4e72b4
https://hg.mozilla.org/mozilla-central/rev/68fad864eca4

Comment 170

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b5bc43016bd
part 67-1: Make `HTMLEditRules::WillDeleteSelection() return `EditActionResult` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/58347c61ae4e
part 67-2: Rename variables in `HTMLEditRules::WillDeleteSelection()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/09cbd757c16b
part 67-3: Split `HTMLEditRules::WillDeleteSelection()` to 3 methods r=m_kato

Comment 171

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/930e13f657a2
part 67-4: Split `HTMLEditRules::HandleDeleteAroundCollapsedSelection()` r=m_kato

Comment 172

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6b5bc43016bd
https://hg.mozilla.org/mozilla-central/rev/58347c61ae4e
https://hg.mozilla.org/mozilla-central/rev/09cbd757c16b

Comment 173

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18c10d385d35
part 67-5: Rewrite `HTMLEditRules::HandleDeleteNonCollapsedSelection()` with `EditorDOMPoint` r=m_kato

Comment 174

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/930e13f657a2
https://hg.mozilla.org/mozilla-central/rev/18c10d385d35

Comment 175

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/349c2ae46aa1
part 67-6: Split the last common part of `HTMLEditRules::HandleDeleteNonCollapsedSelection()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/e790ca6cf615
part 67-7: Make early-return style for first simple deletion case of `HTMLEditRules::HandleDeleteNonCollapsedSelection()` r=m_kato

Comment 176

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23057aa94ccc
part 67-8: Make next simple deletion code use early-return style r=m_kato

Comment 177

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec96ba02db83
part 67-9: Make next early return-style code work without `AutoTrackDOMPoint` r=m_kato

Comment 178

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6db67114e977
part 67-10: Move `HTMLEditRules::WillDeleteSelection()` and related methods to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f6967548beb1
part 68: Make `HTMLEditRules::GetDefinitionListItemTypes()` to a stack class r=m_kato
https://hg.mozilla.org/integration/autoland/rev/e9b7ba436ec2
part 69: Move `HTMLEditRules::PopListItem()` to `HTMLEditor` r=m_kato

Comment 179

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/349c2ae46aa1
https://hg.mozilla.org/mozilla-central/rev/e790ca6cf615

Comment 180

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3764a05b6288
part 70: Move `HTMLEditRules::RemoveListStructur()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c3a59e9eeba8
part 71: Move `HTMLEditRules::WillRemoveList()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/4f6733afb731
part 72: Move `HTMLEditRules::ChangeMarginStart()` to `HTMLEditor` and get rid of its wrappers r=m_kato
https://hg.mozilla.org/integration/autoland/rev/196ce3ca6e9d
part 73: Move `HTMLEditRules::IndentAroundSelectionWithCSS()` to `HTMLEditor` r=m_kato

Comment 181

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/23057aa94ccc
https://hg.mozilla.org/mozilla-central/rev/ec96ba02db83
https://hg.mozilla.org/mozilla-central/rev/6db67114e977
https://hg.mozilla.org/mozilla-central/rev/f6967548beb1
https://hg.mozilla.org/mozilla-central/rev/e9b7ba436ec2
https://hg.mozilla.org/mozilla-central/rev/3764a05b6288
https://hg.mozilla.org/mozilla-central/rev/c3a59e9eeba8
https://hg.mozilla.org/mozilla-central/rev/4f6733afb731
https://hg.mozilla.org/mozilla-central/rev/196ce3ca6e9d

Comment 182

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6611e527537e
part 74: Move `HTMLEditRules::IndentAroundSelectionWithHTML()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/eb2568f1bf7e
part 75: Move `HTMLEditRules::OutdentPartOfBlock()` to `HTMLEditor` r=m_kato

Comment 183

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/6611e527537e
https://hg.mozilla.org/mozilla-central/rev/eb2568f1bf7e

Comment 184

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a139a607ac27
part 76: Move `HTMLEditRules::OutdentAroundSelection()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/9ea3f12721ae
part 77: Move `HTMLEditRules::WillIndent()`, `HTMLEditRules::WillHTMLIndent()` and `HTMLEditRules::WillCSSIndent()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/edaf6d0b4d04
part 78: Move `HTMLEditRules::WillOutdent()` to `HTMLEditor` r=m_kato

Comment 185

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4351a64b385e
part 79: Make `HTMLEditor::IndentAsAction()` and `HTMLEditor::OutdentAsAction()` call specific `AsSubAction()` methods instead of `WillDoAction()` and `DidDoAction()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8c629098c9c9
part 80: Move `HTMLEditRules::AlignBlockContents()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/bc646246502b
part 81: Move `HTMLEditRules::AlignInnerBlocks()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/82d915a1e498
part 82: Move `HTMLEditRules::MakeTransitionList()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/fd513039041e
part 83: Move `HTMLEditRules::MakeSureElemStartsOrEndsOnCR()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/7a484d560816
part 84: Move `HTMLEditRules::RemoveAlignment()` to `HTMLEditor` r=m_kato

Comment 186

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/a139a607ac27
https://hg.mozilla.org/mozilla-central/rev/9ea3f12721ae
https://hg.mozilla.org/mozilla-central/rev/edaf6d0b4d04
https://hg.mozilla.org/mozilla-central/rev/4351a64b385e
https://hg.mozilla.org/mozilla-central/rev/8c629098c9c9
https://hg.mozilla.org/mozilla-central/rev/bc646246502b
https://hg.mozilla.org/mozilla-central/rev/82d915a1e498
https://hg.mozilla.org/mozilla-central/rev/fd513039041e
https://hg.mozilla.org/mozilla-central/rev/7a484d560816

Comment 187

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/220c2d3cd5e0
part 85: Move `HTMLEditRules::AlignBlock()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a25bf7589eb6
part 86: Move `HTMLEditRules::WillAlign()` and `HTMLEditRules::AlignContentsAtSelection()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8edfd974fc63
part 87: Move `HTMLEditRules::SelectionEndpointInNode()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/95684e6acbd3
part 88: Move `HTMLEditRules::FindNearEditableNode()` to `HTMLEditor` r=m_kato

Comment 188

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/735f520eadfa
part 89: Move `HTMLEditRules::AdjustSelection()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/99edaf54b89f
part 90: Move `HTMLEditRules::ConfirmSelectionInBody()` to `HTMLEditor` r=m_kato

Comment 189

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

Attachment: Bug 1574852 - part 96: Merge `HTMLEditRules::DidAbsolutePosition()` with `HTMLEditRules::WillAbsolutePosition()`

Oddly, absolute position is handled as following steps.

1. WillAbsolutePosition() calls PrepareToMakeElementAbsolutePosition()
   to consider the target element.
2. Set TopLevelEditSubActionData::mNewBlockElement to it.
3. DidAbsolutePosition() makes it absolute-positioned.

So that, all of them can be done in WillAbsolutePosition() like other
edit sub-action handling.

Comment 190

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

Attachment: Bug 1574852 - part 97: Move `HTMLEditRules::PrepareToMakeElementAbsolutePosition()` to `HTMLEditor`

Only caller of it is WillRemoveAbsolutePosition() and it always sets
*aHandled to true before calling it. Therefore, it does not need to take
it as an argument.

Comment 191

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

Attachment: Bug 1574852 - part 98: Move `HTMLEditRules::WillAbsolutePosition()` and `HTMLEditRules::WillRemoveAbsolutePosition()` to `HTMLEditor`

Comment 192

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

Attachment: Bug 1574852 - part 99: Move `HTMLEditRules::WillRelativeChangeZIndex()` to `HTMLEditor`

Comment 193

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

Attachment: Bug 1574852 - part 100: Get rid of `TextEditRules::DidDeleteSelection()`

There are only 3 callers and it does simple but different 2 things. One of
the callers is HTMLEditRules::DidDeleteSelection() so that if same things
are done by TextEditor::DeleteSelectionAsSubAction(), it does not need to
duplicate the code. Therefore, we need to duplicate the code into
TextEditor::DeleteSelectionAsSubAction() and TextEditRules::WillSetText().
Then, TextEditRules::WillSetText() can avoid accessing Selection since
it still grabs the modified text node.

Note that only when it's called by TextEditRules::DidDoAction(),
AutoTransactionsConserveSelection has been set. However, neither
DeleteNodeWithTransaction() nor DeleteNodeTransaction::DoTransaction()
changes Selection. Therefore, it hasn't do anything. So, we can remove
it right now.

Comment 194

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

Attachment: Bug 1574852 - part 101: Move `HTMLEditRules::DidDeleteSelection()` to `HTMLEditor`

Comment 195

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

Attachment: Bug 1574852 - part 102: Move `TextEditRules::TruncateInsertionIfNeeded()` to `TextEditor`

Comment 196

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

Attachment: Bug 1574852 - part 103: Move `TextEditRules::WillInsertLineBreak()` to `TextEditor`

Comment 197

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

Attachment: Bug 1574852 - part 104: Move `TextEditRules::WillInsertText()` to `TextEditor` and make `HTMLEditor::WillInsertText()` override it

And also this patch moves TextEditRules::HandleNewLines() and
TextEditRules::DontEchoPassword() to TextEditor.

Comment 198

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

Attachment: Bug 1574852 - part 105: Get rid of `TextEditRules::WillSetProperty()` and `TextEditRules::WillRemoveProperty()`

Oddly, they are used only by HTMLEditor, but implemented by TextEditRules.
They cancels when the editor is in plaintext mode. So, actual things are
implemented by each caller. This patch cleans them up too.

Comment 199

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

Attachment: Bug 1574852 - part 106-1: Move `TextEditRules::WillDeleteSelection()` and `TextEditRules::DeleteSelectionWithTransaction()` to `TextEditor` and make `HTMLEditor::HandleDeleteSelection()` override the former

TextEditor::DeleteSelectionAsSubAction() starts to handle all
"delete selection" sub-actions (i.e., even if the instance is HTMLEditor).
Therefore, TextEditRules::WillDeleteSelection() should be renamed to
TextEditor::HandleDeleteSelection() and we need to make it virtual.

Comment 200

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

Attachment: Bug 1574852 - part 106-2: Move calling `DeleteSelectionWithTransaction()` and `HTMLEditor` specific post handling into `HandleDeleteSelection()` from `TextEditor::DeleteSelectionAsSubAction()`

In TextEditor::HandleDeleteSelection(), we have only one path of returning
EditActionIgnored(). Therefore, it should call
DeleteSelectionWithTransaction() directly.

On the other hand, it's not clear in HTMLEditor::HandleDeleteSelection()
since it may be called recursively by its helper methods. Therefore,
this patch creates HTMLEditor::HandleDeleteSelectionInternal() for the
recursive calls and makes HTMLEditor::HandleDeleteSelection() call
DeleteSelectionWithTransaction() if nobody handled it and to the
HTMLEditor specific post-process in it.

Comment 201

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df2f57c5c750
part 91: Move `HTMLEditRules::InsertBRElementToEmptyListItemsAndTableCellsInRange()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/d76478e79d37
part 92: Move `HTMLEditRules::RemoveEmptyNodesInChangedRange()` to `HTMLEditor` r=m_kato

Comment 202

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/220c2d3cd5e0
https://hg.mozilla.org/mozilla-central/rev/a25bf7589eb6
https://hg.mozilla.org/mozilla-central/rev/8edfd974fc63
https://hg.mozilla.org/mozilla-central/rev/95684e6acbd3
https://hg.mozilla.org/mozilla-central/rev/735f520eadfa
https://hg.mozilla.org/mozilla-central/rev/99edaf54b89f

Comment 203

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/df2f57c5c750
https://hg.mozilla.org/mozilla-central/rev/d76478e79d37

Comment 204

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/36ac44ffe67e
part 93: Move `HTMLEditRules::CheckInterlinePosition()` to `HTMLEditor` r=m_kato

Comment 205

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

Attachment: Bug 1574852 - part 107: Get rid of `TextEditRules::GetTextNodeAroundSelectionStartContainer()` since nobody uses it

Comment 206

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

Attachment: Bug 1574852 - part 108: Move `TextEditRules::WillOutputText()` to `TextEditor`

Comment 207

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

Attachment: Bug 1574852 - part 109: Move `TextEditRules::WillSetText()` to `TextEditor`

And also renaming EditorBase::SetTextImpl().

Comment 208

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

Attachment: Bug 1574852 - part 110: Split `HTMLEditor::WillInsert()`

It does 4 different things so that it looks like a black-box from the
callers.

First, only HTMLEditRules::WillDoAction() refers aCancel out argument.
Therefore, it should check whether it's cancelled or not directly.

Next, EnsureNoPaddingBRElementForEmptyEditor() should be called by each
caller directly.

Then, the renaming part can be split to 2 methods. One is adjusting
caret position and the other preparing inline style for new content.

Unfortunately, this patch makes each caller messy. I think that for the
3rd job (i.e., adjusting caret position), each caller should retrieve the
adjusted caret position and use it directly instead of handling with
Selection in the future.

Comment 209

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

Attachment: Bug 1574852 - part 111: Make methods calling `HTMLEditRules::WillDoAction()` with `EditSubAction::eInsertElement` stop using it

Unfortunately, EditSubAction::eInsertElement is used by 4 methods and one
of them is too big method. But we should move what old WillInsert() does
into them for stop making anybody use HTMLEditRules.

Comment 210

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

Attachment: Bug 1574852 - part 112: Make `TextEditor::InsertWithQuotationsAsSubAction()` virtual and `HTMLEditor` override it

It requires different preparation in TextEditor and HTMLEditor but it's
not split. Therefore, we should make it virtual and override it to use
different preparation code. Fortunately, its code is enough simple to
duplicate.

Additionally, this removes unnecessary code from TextEditRules and
HTMLEditRules including WillDoAction() and DidDoAction()!

Comment 211

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

Attachment: Bug 1574852 - part 113: Move `TextEditRules::CreateTrailingBRIfNeeded()` and `TextEditRules::CollapseSelectionToTrailingBRIfNeeded()` to `TextEditor`

Comment 212

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

That's all!

Unfortunately, HTMLEditRules::WillInsert() related part makes the callers messy even though making them easier to understand. I'll try to think how we should do between "making sure everybody does all things at preparation" vs. "avoiding too generic method name".

Comment 213

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/36ac44ffe67e

Comment 214

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d41ae182072c
part 94: Move `HTMLEditRules::PinSelectionToNewBlock()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f59ebdb53f9a
part 95: Move `HTMLEditRules::AfterEditInner()` to `HTMLEditor` r=m_kato

Comment 215

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/d41ae182072c
https://hg.mozilla.org/mozilla-central/rev/f59ebdb53f9a

Comment 216

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d85fd47dbcc7
part 96: Merge `HTMLEditRules::DidAbsolutePosition()` with `HTMLEditRules::WillAbsolutePosition()` r=m_kato

Comment 217

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/663df481fcbf
part 97: Move `HTMLEditRules::PrepareToMakeElementAbsolutePosition()` to `HTMLEditor` r=m_kato

Comment 218

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11b6d1bbafe7
part 98: Move `HTMLEditRules::WillAbsolutePosition()` and `HTMLEditRules::WillRemoveAbsolutePosition()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/dc0f654c9ccd
part 99: Move `HTMLEditRules::WillRelativeChangeZIndex()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/886ccc65f1c9
part 100: Get rid of `TextEditRules::DidDeleteSelection()` r=m_kato

Comment 219

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d85fd47dbcc7
https://hg.mozilla.org/mozilla-central/rev/663df481fcbf

Comment 220

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b60769450e9d
part 101: Move `HTMLEditRules::DidDeleteSelection()` to `HTMLEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/123b06288aa0
part 102: Move `TextEditRules::TruncateInsertionIfNeeded()` to `TextEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/61cc90ceac61
part 103: Move `TextEditRules::WillInsertLineBreak()` to `TextEditor` r=m_kato

Comment 221

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/11b6d1bbafe7
https://hg.mozilla.org/mozilla-central/rev/dc0f654c9ccd
https://hg.mozilla.org/mozilla-central/rev/886ccc65f1c9

Comment 222

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/b60769450e9d
https://hg.mozilla.org/mozilla-central/rev/123b06288aa0
https://hg.mozilla.org/mozilla-central/rev/61cc90ceac61

Comment 223

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1bc2978543d1
part 104: Move `TextEditRules::WillInsertText()` to `TextEditor` and make `HTMLEditor::WillInsertText()` override it r=m_kato
https://hg.mozilla.org/integration/autoland/rev/41bc3a403c31
part 105: Get rid of `TextEditRules::WillSetProperty()` and `TextEditRules::WillRemoveProperty()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/cbec1b8d83ed
part 106-1: Move `TextEditRules::WillDeleteSelection()` and `TextEditRules::DeleteSelectionWithTransaction()` to `TextEditor` and make `HTMLEditor::HandleDeleteSelection()` override the former r=m_kato

Comment 224

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e5770e71acd5
part 106-2: Move calling `DeleteSelectionWithTransaction()` and `HTMLEditor` specific post handling into `HandleDeleteSelection()` from `TextEditor::DeleteSelectionAsSubAction()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c915823b6b93
part 107: Get rid of `TextEditRules::GetTextNodeAroundSelectionStartContainer()` since nobody uses it r=m_kato
https://hg.mozilla.org/integration/autoland/rev/d9f9610b4d24
part 108: Move `TextEditRules::WillOutputText()` to `TextEditor` r=m_kato

Comment 225

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/1bc2978543d1
https://hg.mozilla.org/mozilla-central/rev/41bc3a403c31
https://hg.mozilla.org/mozilla-central/rev/cbec1b8d83ed

Comment 226

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/e5770e71acd5
https://hg.mozilla.org/mozilla-central/rev/c915823b6b93
https://hg.mozilla.org/mozilla-central/rev/d9f9610b4d24

Comment 227

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2b6968592570
part 109: Move `TextEditRules::WillSetText()` to `TextEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/f54f6af6359d
part 110: Split `HTMLEditor::WillInsert()` r=m_kato

Comment 228

[Raul Gurzau (:RaulG)]

Backed out 2 changesets (bug 1574852) for assertion failures at TextEditRules.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/aa6d5d26e1d2ceb03b066e33e22b4b4059ff5102

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=266721422&resultStatus=testfailed%2Cbusted%2Cexception&revision=f54f6af6359dc358b2b91f6870f418462cc31e88

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266721422&repo=autoland&lineNumber=4082

Log snippet:
[task 2019-09-14T23:20:11.700Z] 23:20:11 INFO - TEST-START | browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html
[task 2019-09-14T23:20:11.761Z] 23:20:11 INFO - GECKO(2372) | ++DOMWINDOW == 13 (0x7f2b0641a800) [pid = 2445] [serial = 20] [outer = 0x7f2b035fa3e0]
[task 2019-09-14T23:20:12.123Z] 23:20:12 INFO - GECKO(2372) | --DOMWINDOW == 12 (0x7f2b07036000) [pid = 2445] [serial = 15] [outer = (nil)] [url = http://mochi.test:8888/tests/SimpleTest/iframe-between-tests.html]
[task 2019-09-14T23:20:12.124Z] 23:20:12 INFO - GECKO(2372) | --DOMWINDOW == 11 (0x7f2b02d70000) [pid = 2445] [serial = 11] [outer = (nil)] [url = http://mochi.test:8888/tests/browser/extensions/formautofill/test/mochitest/test_address_level_1_submission.html]
[task 2019-09-14T23:20:12.124Z] 23:20:12 INFO - GECKO(2372) | --DOMWINDOW == 10 (0x7f2b038e4000) [pid = 2445] [serial = 14] [outer = (nil)] [url = http://mochi.test:8888/]
[task 2019-09-14T23:20:13.106Z] 23:20:13 INFO - GECKO(2372) | --DOMWINDOW == 11 (0x7f70a3b24000) [pid = 2422] [serial = 2] [outer = (nil)] [url = about:blank]
[task 2019-09-14T23:20:13.106Z] 23:20:13 INFO - GECKO(2372) | --DOMWINDOW == 10 (0x7f70a3c8b400) [pid = 2422] [serial = 4] [outer = (nil)] [url = about:blank]
[task 2019-09-14T23:20:13.107Z] 23:20:13 INFO - GECKO(2372) | --DOMWINDOW == 9 (0x7f70a3c8d800) [pid = 2422] [serial = 6] [outer = (nil)] [url = about:blank]
[task 2019-09-14T23:20:13.107Z] 23:20:13 INFO - GECKO(2372) | --DOMWINDOW == 8 (0x7f70a3c90000) [pid = 2422] [serial = 8] [outer = (nil)] [url = about:blank]
[task 2019-09-14T23:20:13.155Z] 23:20:13 INFO - GECKO(2372) | --DOMWINDOW == 7 (0x7f70a4b835c0) [pid = 2422] [serial = 7] [outer = (nil)] [url = moz-extension://cc12f14d-a078-467f-925e-f03435f1d5fb/_generated_background_page.html]
[task 2019-09-14T23:20:14.639Z] 23:20:14 INFO - GECKO(2372) | Assertion failure: !AsTextEditor(), at /builds/worker/workspace/build/src/editor/libeditor/TextEditRules.cpp:684
[task 2019-09-14T23:20:35.543Z] 23:20:35 INFO - GECKO(2372) | #01: mozilla::TextEditor::SetTextAsSubAction(nsTSubstring<char16_t> const&) [editor/libeditor/TextEditor.cpp:1067]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #02: mozilla::TextEditor::SetTextAsAction(nsTSubstring<char16_t> const&, nsIPrincipal*) [editor/libeditor/TextEditor.cpp:993]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #03: nsTextEditorState::SetValue(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const*, unsigned int) [dom/html/nsTextEditorState.cpp:2394]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #04: mozilla::dom::HTMLInputElement::SetValueInternal(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const*, unsigned int) [dom/html/HTMLInputElement.cpp:2644]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #05: mozilla::dom::HTMLInputElement::SetValue(nsTSubstring<char16_t> const&, mozilla::dom::CallerType, mozilla::ErrorResult&) [dom/html/HTMLInputElement.cpp:1607]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #06: mozilla::dom::HTMLInputElement_Binding::set_value(JSContext*, JS::Handle<JSObject*>, mozilla::dom::HTMLInputElement*, JSJitSetterCallArgs) [s3:gecko-generated-sources:574d6ddfcd438d58aabc0df938153eb30193fcbbd96ac311a4e58c0b296f81393cb4bbe3106b0cb067e7e404c199c04df78d41e0c5eafdadbbb0750db214ede6/dom/bindings/HTMLInputElementBinding.cpp::3091]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #07: bool mozilla::dom::binding_detail::GenericSetter<mozilla::dom::binding_detail::NormalThisPolicy>(JSContext*, unsigned int, JS::Value*) [dom/bindings/BindingUtils.cpp:3121]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #08: CallJSNative(JSContext*, bool ()(JSContext, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) [js/src/vm/Interpreter.cpp:458]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #09: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:551]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #10: js::CallSetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) [js/src/vm/Interpreter.cpp:775]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO - GECKO(2372) | #11: SetExistingProperty(JSContext*, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<js::NativeObject*>, JS::Handle<JS::PropertyResult>, JS::ObjectOpResult&) [js/src/vm/NativeObject.cpp:2932]
[task 2019-09-14T23:20:35.544Z] 23:20:35 INFO -

Comment 229

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

Oh, I must have fixed the wrong assertion in a wrong patch....

Comment 230

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6c2a63934fb
part 109: Move `TextEditRules::WillSetText()` to `TextEditor` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/28c561e18cab
part 110: Split `HTMLEditor::WillInsert()` r=m_kato

Comment 231

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c4b35bfd9276
part 111: Make methods calling `HTMLEditRules::WillDoAction()` with `EditSubAction::eInsertElement` stop using it r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a6c31b6a60e0
part 112: Make `TextEditor::InsertWithQuotationsAsSubAction()` virtual and `HTMLEditor` override it r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a4bfd6e22c41
part 113: Move `TextEditRules::CreateTrailingBRIfNeeded()` and `TextEditRules::CollapseSelectionToTrailingBRIfNeeded()` to `TextEditor` r=m_kato

Comment 232

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/e6c2a63934fb
https://hg.mozilla.org/mozilla-central/rev/28c561e18cab

Comment 233

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/c4b35bfd9276
https://hg.mozilla.org/mozilla-central/rev/a6c31b6a60e0
https://hg.mozilla.org/mozilla-central/rev/a4bfd6e22c41