Closed Bug 1574852 Opened 7 months ago Closed 6 months ago

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

Categories

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

task

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

Attachments

(123 files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

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.

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.

Keywords: leave-open

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

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.

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

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.

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().

Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/bfbdc9ba712c
part 4: Move `HTMLEditRules::GetInlineStyles()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6456b7a6cc67
part 5: Move `HTMLEditRules::ReapplyCachedStyles()` to `HTMLEditor` r=m_kato

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.

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.

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.

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().

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

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".

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.

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

Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9a87c2e0fea5
part 5: Move `HTMLEditRules::ReapplyCachedStyles()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/43bfa576a15e
part 8: Move `HTMLEditRules::WillInsert()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/da6c34e227b2
part 9: Move `HTMLEditRules::CreateStyleForInsertText()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/12ec2813293d
part 10: Move `HTMLEditRules::WillInsertText()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/4ab23a7388e2
part 11: Move `HTMLEditRules::GetTopEnclosingMailCite()` to `HTMLEditor` r=m_kato

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.

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/41bb079385e1
part 12: Move `HTMLEditRules::SplitMailCites()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/236e278e33a4
part 13: Move `HTMLEditRules::CanContainParagraph()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/f41a0d889470
part 14: Move `HTMLEditRules::InsertBRElement()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3a2413d91a19
part 15: Move `HTMLEditUtils::GetHighestInlineParent()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/6174156c2fff
part 16: Move `HTMLEditRules::BustUpInlinesAtRangeEndpoints()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/fa5c785f9005
part 17: Move `HTMLEditRules::GetInnerContent()` to `HTMLEditor` r=m_kato
Flags: needinfo?(m_kato)
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/b3dc91a89d99
part 18: Move `HTMLEditRules::BustUpInlinesAtBRs()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/166c445265ff
part 19: Move `HTMLEditRules::GetNodesForOperation()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/c980a90fce4f
part 20: Move `HTMLEditRules::GetPromotedPoint()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/cd856fd9fc39
part 21: Move `HTMLEditRules::PromoteRange()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/c5adf2e84f97
part 22: Move `HTMLEditRules::GetPromotedRanges()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/19803f3decdd
part 23: Move `HTMLEditRules::GetNodesFromSelection()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/d1df6caf3b1d
part 24: Move `HTMLEditRules::GetNodesFromPoint()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/2dfef8511618
part 25: Move `HTMLEditRules::GetChildNodesForOperation()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/74ce25be0e9a
part 26: Move a part of `HTMLEditRules::LookInsideDivBQandList()` and remove the others r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ada62f7a9760
part 27: Move first half of `HTMLEditRules::GetListActionNodes()` to `HTMLEditor` and each caller r=m_kato
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/18659 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Upstream PR merged by moz-wptsync-bot
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/d9b024555ac1
part 38: Move `HTMLEditRules::MakeBasicBlock()` to `HTMLEditor` r=m_kato
Priority: -- → P3

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

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

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

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().

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

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.

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.

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.

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

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.

Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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

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

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().

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

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.

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.

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/99ec3f15612f
part 47: Move `HTMLEditRules::ReturnInParagraph()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/15b069730cb6
part 48: Move `HTMLEditRules::ReturnInHeader()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/0c0302b3db46
part 55: Move `HTMLEditRules::MakeList()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/082159fea413
part 60: Move `HTMLEditRules::MoveNodeSmart()` and `HTMLEditRules::MoveContents()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/cd2b83d5dda7
part 61: Move `HTMLEditRules::MoveBlock()` to `HTMLEditor` r=m_kato

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.

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.

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

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

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

Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/930e13f657a2
part 67-4: Split `HTMLEditRules::HandleDeleteAroundCollapsedSelection()` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/18c10d385d35
part 67-5: Rewrite `HTMLEditRules::HandleDeleteNonCollapsedSelection()` with `EditorDOMPoint` r=m_kato
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/23057aa94ccc
part 67-8: Make next simple deletion code use early-return style r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/ec96ba02db83
part 67-9: Make next early return-style code work without `AutoTrackDOMPoint` r=m_kato
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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

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.

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.

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.

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

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.

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.

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.

Pushed by masayuki@d-toybox.com:
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
Regressions: 1580097
No longer regressions: 1580097
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/36ac44ffe67e
part 93: Move `HTMLEditRules::CheckInterlinePosition()` to `HTMLEditor` r=m_kato

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.

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.

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()!

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".

Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/d85fd47dbcc7
part 96: Merge `HTMLEditRules::DidAbsolutePosition()` with `HTMLEditRules::WillAbsolutePosition()` r=m_kato
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/663df481fcbf
part 97: Move `HTMLEditRules::PrepareToMakeElementAbsolutePosition()` to `HTMLEditor` r=m_kato
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Regressions: 1581337
Pushed by masayuki@d-toybox.com:
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

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 -

Flags: needinfo?(masayuki)

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

Pushed by masayuki@d-toybox.com:
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
Pushed by masayuki@d-toybox.com:
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
Flags: needinfo?(masayuki)
Keywords: leave-open
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.