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.
Differential Revision: https://phabricator.services.mozilla.com/D44199
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D44197
--HG--
extra : moz-landing-system : lando
`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`.
Differential Revision: https://phabricator.services.mozilla.com/D44195
--HG--
extra : moz-landing-system : lando
Additionally, this patch makes it use early-return style with `continue` as
far as making number of changing line minimized.
Differential Revision: https://phabricator.services.mozilla.com/D44194
--HG--
extra : moz-landing-system : lando
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()`.
Differential Revision: https://phabricator.services.mozilla.com/D44190
--HG--
extra : moz-landing-system : lando
It's used both by `TextEditRules` and `HTMLEditRules` so that `EditorBase`
should have it instead.
Differential Revision: https://phabricator.services.mozilla.com/D44188
--HG--
extra : moz-landing-system : lando
`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()`
Differential Revision: https://phabricator.services.mozilla.com/D44179
--HG--
extra : moz-landing-system : lando
And this fixes the caller which has not guaranteed the lifetime of the
start container.
Differential Revision: https://phabricator.services.mozilla.com/D44175
--HG--
extra : moz-landing-system : lando
Since bug 1577443 is landed in comm-central, no one uses `nsIHTMLEditor.getLinkedObject`.
Differential Revision: https://phabricator.services.mozilla.com/D44361
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D43190
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::GetListActionNodes()` removes non-editable element. However,
this should've been done by collector methods.
Differential Revision: https://phabricator.services.mozilla.com/D43026
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D43024
--HG--
extra : moz-landing-system : lando
`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".
Differential Revision: https://phabricator.services.mozilla.com/D43023
--HG--
extra : moz-landing-system : lando
It just collects all children of given node so that it can be a static method
in `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D43022
--HG--
extra : moz-landing-system : lando
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()`.
Differential Revision: https://phabricator.services.mozilla.com/D43021
--HG--
extra : moz-landing-system : lando
`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`.
Differential Revision: https://phabricator.services.mozilla.com/D43020
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D43018
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D43017
--HG--
extra : moz-landing-system : lando
`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 `GetCurrentHardLineStartPoint()`. The last
one is for expanding end of the range to end of its line. It's named as
`GetCurrentHardLineEndPoint()`.
Differential Revision: https://phabricator.services.mozilla.com/D42791
--HG--
extra : moz-landing-system : lando
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::SplitInlinesAndCollectEditTargetNodes()`. 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.
Differential Revision: https://phabricator.services.mozilla.com/D42790
--HG--
extra : moz-landing-system : lando
The fix here is to first check `NS_FAILED(rv)`, because if that's the
case, `cancel` wasn't necessarily set to a value.
As best practice I initialized `cancel` and `handled` with default
values.
Differential Revision: https://phabricator.services.mozilla.com/D43071
--HG--
extra : moz-landing-system : lando
