This is related to bug 1530649. When using <span> element with contentedtiable,
we won't insert <br> element at last. When Korean IME on macOS commits
composition by space key, composition string has space.
Gekco removes U+0020 space when it is last character into editing host. To keep
whitespace, we have to replace with NBSP when it is last.
Differential Revision: https://phabricator.services.mozilla.com/D48146
--HG--
extra : moz-landing-system : lando
Both method take a DOM point with `nsCOMPtr<nsINode>*` and `int32_t*`.
This makes each caller complicated. Instead, we should use stack only class
to return both `EditorDOMPoint` and `nsresult`. I name it `EditResult`.
Additionally, this fixes a bug of `HTMLeditor::SplitStyleAboveRange()`. That
is not tracking new selection start point while it splits elements at end
of given range. This is detected by the debug assertion in
`ToRawRangeBoundary()` (i.e., this fix is required to pass some tests).
Differential Revision: https://phabricator.services.mozilla.com/D47858
--HG--
extra : moz-landing-system : lando
Most of these tests have been disabled for a long time; they run well
in the current test environment.
Differential Revision: https://phabricator.services.mozilla.com/D47616
--HG--
extra : moz-landing-system : lando
Instead, rely on the proper initialization of `mWrapColumn` via
`nsPlainTextSerializer::Init`.
This will allow to move `mWrapColumn` to `nsPlainTextSerializer::Settings::mWrapColumn`,
simplifying reasoning about it.
Differential Revision: https://phabricator.services.mozilla.com/D46129
--HG--
extra : moz-landing-system : lando
Currently, we can use chrome process's shortcut key with
`EventUtils.synthesizeKey()` with enabling `"test.events.async.enabled"` pref.
So, we should reconvert it to a mochitest for making it more stable.
Oddly, when I try to run this test as test-verify on macOS, it permanently
fails rendering resizer of `<textarea>` elements immediately after creation.
Therefore, this patch disables this test in test-verify on macOS.
Differential Revision: https://phabricator.services.mozilla.com/D46578
--HG--
rename : editor/libeditor/tests/browser_bug629172.js => editor/libeditor/tests/test_bug629172.html
extra : moz-landing-system : lando
Only for solving the order of `AutoEditInitRulesTrigger` destruction when
`HTMLEditor::Init()` calls `TextEditor::Init()`, `TextEditor` has unnecessary
counter and the initialization code is made harder to understand.
Therefore, this patch makes `TextEditor::Init()` and `HTMLEditor::Init()`
directly call `InitEditorContentAndSelection()` at appropriate time.
Additionally, `HTMLEditor::Init()` can call `EditorBase::Init()` instead of
`TextEditor::Init()` since `TextEditor::Init()` does nothing for `HTMLEditor`
actually.
Differential Revision: https://phabricator.services.mozilla.com/D45793
--HG--
extra : moz-landing-system : lando
Now, we can get rid of `TextEditRules` and `HTMLEditRules` completely.
And also this patch renames their cpp files to `TextEditSubActionHandler`
and `HTMLEditSubActionHandler`.
`TextEditor::Init()` and `HTMLEditor::Init()` are still complicated due to
`AutoEditInitRulesTrigger`. The following patch will remove it.
Differential Revision: https://phabricator.services.mozilla.com/D45792
--HG--
rename : editor/libeditor/HTMLEditRules.cpp => editor/libeditor/HTMLEditSubActionHandler.cpp
rename : editor/libeditor/TextEditRules.cpp => editor/libeditor/TextEditSubActionHandler.cpp
extra : moz-landing-system : lando
It takes a lot of `bool` out arguments. Therefore, we should make it a
stack only class and caller should retrieve only necessary information.
Differential Revision: https://phabricator.services.mozilla.com/D45787
--HG--
extra : moz-landing-system : lando
And also this patch make each `AutoEditSubActionNotifier` creator check
the result of `OnStartToHandleTopLevelEditSubAction()` at least for
`NS_ERROR_EDITOR_DESTROYED`.
We need to take care of its destructor's result later, though.
Differential Revision: https://phabricator.services.mozilla.com/D45786
--HG--
extra : moz-landing-system : lando
This is regression by bug 1530649.
After landing bug 1530649, we try to scan end point of replacement text. But
in this bug's situation, afterRun becomes same as current ws run by landing
bug 1530649. To get white space type of next of replacement end, we have to
scan around end point again.
Differential Revision: https://phabricator.services.mozilla.com/D45947
--HG--
extra : moz-landing-system : lando
`TextEditRules::DocumentIsEmpty()` is a wrapper of `TextEditor::IsEmpty()` so
that we can get rid of it simply.
`HTMLEditRules::DocumentIsEmpty()` needs to change. It's oddly checks only
`EditorBase::mPaddingBRElementForEmptyEditor` is `nullptr` or not. However,
the editor may be completely empty. And the result may be different from
`TextEditor::IsEmpty()` which is not overridden by `HTMLEditor`. For partially
solving this issue, this patch makes `HTMLEditor` overrides `IsEmpty()` and
optimizes `TextEditor::IsEmpty()`.
With this change, the caller of `HTMLEditRules::DocumentIsEmpty()` may behave
differently in the only caller, `HTMLEditor::SelectEntireDocument()`. And
unfortunately, its root called from `SelectAllCommand::DoCommand()`. However,
it does just collapse `Selection` into the root element (`<body>` or
`Document.documentElement`) if it returns `true`. Therefore, this change
must be safe since anyway `SelectionRefPtr()->SelectAllChildren()` with
the root element is exactly same as `SelectionRefPtr()->Collapse()` with
the empty root element if it's truly empty.
Differential Revision: https://phabricator.services.mozilla.com/D45784
--HG--
extra : moz-landing-system : lando
The main purpose of them is modifying
`TopLevelEditSubActionData::mChangedRange`. Therefore, they should be in the
struct.
Differential Revision: https://phabricator.services.mozilla.com/D45783
--HG--
extra : moz-landing-system : lando
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()`!
Differential Revision: https://phabricator.services.mozilla.com/D45495
--HG--
extra : moz-landing-system : lando
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`.
Differential Revision: https://phabricator.services.mozilla.com/D45494
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D45493
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D45493
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D45301
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D45300
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D45299
--HG--
extra : moz-landing-system : lando
And also this patch moves `TextEditRules::HandleNewLines()` and
`TextEditRules::DontEchoPassword()` to `TextEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D45298
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D45294
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D45291
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D45290
--HG--
extra : moz-landing-system : lando
And also this splits large 2 blocks of
`HTMLEditRules::AlignContentsAtSelection()` to 2 methods.
Differential Revision: https://phabricator.services.mozilla.com/D44790
--HG--
extra : moz-landing-system : lando
With guaranteeing the argument element type with `MOZ_ASSERT()`, we can
make it really simpler.
Differential Revision: https://phabricator.services.mozilla.com/D44789
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D44787
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D44781
--HG--
extra : moz-landing-system : lando
Remove test manifest annotations that specifically target fennec,
or likely target the android 4.3 emulator.
Differential Revision: https://phabricator.services.mozilla.com/D45018
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D44461
--HG--
extra : moz-landing-system : lando
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.
Differential Revision: https://phabricator.services.mozilla.com/D44459
--HG--
extra : moz-landing-system : lando
Before splitting the method, we should make it use `EditorDOMPoint` to
treat selection edges.
Differential Revision: https://phabricator.services.mozilla.com/D44455
--HG--
extra : moz-landing-system : lando
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()`.
Differential Revision: https://phabricator.services.mozilla.com/D44453
--HG--
extra : moz-landing-system : lando
Before moving the largest method in our tree, we should split it. This is
first preparation of that.
Differential Revision: https://phabricator.services.mozilla.com/D44451
--HG--
extra : moz-landing-system : lando
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`.
Differential Revision: https://phabricator.services.mozilla.com/D44205
--HG--
extra : moz-landing-system : lando
Additionally, `WSRunObject::ScrabBlockBoundary()` and
`WSRunObject::PreparetToJoinBlocks()` are used only the it. Therefore, we
can clean them up.
Differential Revision: https://phabricator.services.mozilla.com/D44201
--HG--
extra : moz-landing-system : lando
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
This is regression by bug 1543312.
By bug 1543312, we destroy editor when destroying PresShell. But when destroying PresShell, editor doesn't remove anonymous content of editing UI from PresShell. Then, when destroying the frame manager in `PresShell::Destroy`, it hits assertion due to uncomposed doc.
We shouldn't hide editing UI during destroying PresShell and we should hide it after destroyed.
Differential Revision: https://phabricator.services.mozilla.com/D42773
--HG--
extra : moz-landing-system : lando
It's called immediately before setting `mHTMLEditor` and sets `mHTMLEditor` to
`nullptr`. So, it does nothing actually. We can get rid of it.
Differential Revision: https://phabricator.services.mozilla.com/D42771
--HG--
extra : moz-landing-system : lando
`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.
Differential Revision: https://phabricator.services.mozilla.com/D42770
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::IsBlockNode()` just wraps `HTMLEditor::NodeIsBlockStatic()`
and all its users will be moved into `HTMLEditor`. Therefore, we should
unwrap it.
Differential Revision: https://phabricator.services.mozilla.com/D42769
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::mReturnInEmptyLIKillsList` stores value of
`editor.html.typing.returnInEmptyListItemClosesList` at construction, and it's
set to true unless the pref value is `"false"`. However, this pref isn't
registered in anywhere (all.js, firefox.js, etc) nor used in comm-central
and BlueGriffon.
Even if I search the pref in the web, I don't see any information so that
this hidden pref must not be used anybody. Therefore, this patch just removes
this member.
Differential Revision: https://phabricator.services.mozilla.com/D42262
--HG--
extra : moz-landing-system : lando
This patch includes a logic change. `HTMLEditRules::mListenerEnabled` is used
for storing 2 state. One is that whether `HTMLEditRules` can handle the legacy
listener methods (they came from `nsIEditActionListener`) with `mHTMLEditor` or
not. The other is that whether current edit sub-action handler temporarily
disables the listeners for performance or not.
For the former, we can check same result with `HTMLEditRules::mInitialized`
and `HTMLEditRules::mHTMLEditor`.
However, the latter, current design is obviously wrong. Currently,
`HTMLEditRules::mListenerEnabled` is set only to `false` temporarily from
`WillInsertText()`, but the state will be affected to other edit sub-actions
which are nested by mutation event listeners. So, currently,
`HTMLEditRules::mDocChangeRange` may not contain modified range caused by
nested edit sub-actions.
For solving this bug, this patch moves it into `EditSubActionData` rather
than `HTMLEditor` and `TopLevelEditSubActionData`.
Differential Revision: https://phabricator.services.mozilla.com/D42107
--HG--
extra : moz-landing-system : lando
The members of `HTMLEditRules` which are used only while `WillDoAction()` and
`DidDoAction()` are called should be moved to specific stack only struct
`EditorBase::EditSubActionData`.
Differential Revision: https://phabricator.services.mozilla.com/D42106
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::mUtilRange` is used only for the argument of
`HTMLEditRules::UpdateDocChangeRange()`. However, it does not need to be
a range instance since it compares its start and
`TopLevelEditSubAction::mChangedRange->StartRef()`, and its end and
`TopLevelEditSubAction::mChangedRange->EndRef()`. Therefore, with rewriting
it as taking 2 `EditorRawDOMPoint`s, we don't need `mUtilRange`.
Differential Revision: https://phabricator.services.mozilla.com/D42105
--HG--
extra : moz-landing-system : lando
This patch makes `StyleCache` not inherit `ItemProp` because `MOZ_COUNT_CTOR`
and `MOZ_COUNT_DTOR` do not work well with `AutoTArray` and there is no reason
to do that since nobody treat `StyleCache` instance with `ItemProp` pointer.
Differential Revision: https://phabricator.services.mozilla.com/D42103
--HG--
extra : moz-landing-system : lando
For avoiding allocation cost of `RangeItem`, it should be stored by `HTMLEditor`
and reused by all `TopLevelEditSubActionData` instances for the editor instance.
Differential Revision: https://phabricator.services.mozilla.com/D42102
--HG--
extra : moz-landing-system : lando
Most member variables of `HTMLEditRules` are temporary used while between
`BeforeEdit()` and `AfterEdit()` are called or `WillDoAction()` and
`DidDoAction()` are called.
The former means the data should be stored during top-level edit sub-action
is set. Therefore, this patch creates a struct, `TopLevelEditSubActionData`
and make it a member of `AutoEditActionDataSetter`. Then, makes
`EditorBase::TopLevelEditActionDataRef()` return reference of it.
And also this patch moves `HTMLEditRules::mDidDeleteSelection` into it.
Differential Revision: https://phabricator.services.mozilla.com/D42096
--HG--
extra : moz-landing-system : lando
This condition was needed when FindInsertionPrevSibling and co didn't understand
display: contents.
Editor is pretty broken (and calls into PresShell::ContentRemoved directly, and
incorrectly, using anonymous nodes).
In this case we were taking the XBL path because of display: contents, which
means that we tried to seek to the editor anonymous node, and crash (since it's
not an explicit kid).
Editor needs to get fixed, but this is technically more correct and fixes the
crash, so we may as well take it in the interim.
Differential Revision: https://phabricator.services.mozilla.com/D42472
--HG--
extra : moz-landing-system : lando
They're bad, specially if they do vastly different thing in overloaded
functions, like aUseSystemPrincipal and aIsPreload. :)
Differential Revision: https://phabricator.services.mozilla.com/D40851
--HG--
extra : moz-landing-system : lando
`TextEditRules::mDidExplicitlySetInterline` is set to true only by
`HTMLEditRules`, but `TextEditRules::DidDeleteSelection()` refers it.
So, it's enough to make `TextEditRules::DidDeleteSelection()` take the
value and we can move it into `HTMLEditRules`.
Differential Revision: https://phabricator.services.mozilla.com/D41401
--HG--
extra : moz-landing-system : lando
`TextEditRules::mDeleteBidiImmediately` is cache of
`bidi.edit.delete_immediately` pref value at creation time of `TextEditRules`.
However, this is referred when user removes selection. So, there is no
reason to keep same behavior starting from editor creation. In other words,
it must be better to take same behavior in all editor instances.
Therefore, we should remove it and the pref value should be referred directly
when user tries to remove selection.
Differential Revision: https://phabricator.services.mozilla.com/D41400
--HG--
extra : moz-landing-system : lando
`TextEditRules::BeforeEdit()`, `TextEditRules::AfterEdit()`,
`HTMLEditRules::BeforeEdit()` and `HTMLEditRules::AfterEdit()` are always
called with same values as the result of
`EditorBase::GetTopLevelEditSubAction()` and
`EditorBase::GetDirectionOfTopLevelEditSubAction()`.
For making what they do clearer, we should make them access with those
`EditorBase` members for now. This makes those methods ugly due to increasing
number of long lines. However, this issue should be solved when we move them
into `TextEditor` and `HTMLEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D41399
--HG--
extra : moz-landing-system : lando
`TextEditRules::mLockRulesSniffing` is set by `AutoLockRulesSniffing`.
It's created during `BeforeEdit()` and `AfterEdit()` are called, and
`HTMLEditRules::Init()` is initializing `mDocChangeRange`.
The purpose of it is, preventing `BeforeEdit()` and `AfterEdit()` to do
something in that time. For the former cases, we don't need this member
anymore since they won't be nested. Therefore, we need to manage
`HTMLEditRules::BeforeEdit()` and `HTMLEditRules::AfterEdit()` won't do
anything only while `HTMLEditRules::Init()` is called. Therefore,
there should be only `HTMLEditRules::mInitialized` instead.
Differential Revision: https://phabricator.services.mozilla.com/D41398
--HG--
extra : moz-landing-system : lando
`TextEditRules::BeforeEdit()`, `TextEditRules::AfterEdit()`,
`HTMLEditRules::BeforeEdit()` and `HTMLEditRules::AfterEdit()` manages
`TextEditRules::mActionNesting` for preventing that they won't do same thing
per top-level edit sub-action. However, this has already been checked by
their caller, `AutoTopLevelEditSubActionNotifier`. So, we can get rid of it.
Then, `TextEditRules::mTopLevelEditSubAction` is also always same as
`EditorBase::GetTopLevelEditSubAction()`. Therefore, we can get rid of it
too.
Differential Revision: https://phabricator.services.mozilla.com/D41397
--HG--
extra : moz-landing-system : lando
Despite of their names, `TextEditRules::mCachedSelectionNode` and
`TextEditRules::mCachedSelectionOffset` are used only for calling
`EditorBase::HandleInlineSpellCheck()` so that they should be renamed to
explain the purpose.
Additionally, they are not necessary to be in the heap since they are
necessary until `TextEditRules::AfterEdit()` is called. Therefore, we can
move them into `EditorBase::HandleInlineSpellCheck()`.
Finally, `TextEditRules::BeforeEdit()` and `TextEditRules::AfterEdit()` are
called only by `TextEditor::OnStartToHandleTopLevelEditSubAction()` and
`TextEditor::OnEndHandlingTopLevelEditSubAction()`. Therefore, we can move
the setter to `TextEditor::OnStartToHandleTopLevelEditSubAction()`.
Differential Revision: https://phabricator.services.mozilla.com/D41396
--HG--
extra : moz-landing-system : lando
When browsing reported site (https://minecraft.curseforge.com/), user that uses Android cannot set caret on some editable contents.
GV's IME code uses `NOTIFY_IME_OF_SELECTION_CHANGE` to update selection on native IME. But GV doesn't receives this notification on this reported site. This notification is fired by editor's selection listener, but when this occurs, this notification is no logner fired unfortunately. Because selection listener isn't registered by current selection.
At first, editor registers selection listener by `EditorBase::Init`. But like test case, if PresShell is destroyed after editor is created, this registration is no longer used, and we have to register it by new selection again. So we should tear down editor when PresShell is destroyed. (or we have to add a way to re-initialize selection listener?)
This isn't related to Android. Android's IME code depends on this notification, so this occurs easily.
Also, `PresShell::SetCaretEnabled` only works when caret is valid (`PresShell::GetCaret` has caret). If caret is nothing, it hits assertion.
And, attached test case is reproduced sample.
Differential Revision: https://phabricator.services.mozilla.com/D41356
--HG--
extra : moz-landing-system : lando
`TextEditRules` and `HTMLEditRules` still refer
`EditorBase::mPaddingBRElementForEmptyEditor` directly but this is really ugly.
Therefore, this patch creates `EditorBase::HasPaddingBRElementForEmptyEditor()`
for wrapping its access.
Differential Revision: https://phabricator.services.mozilla.com/D41162
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::OnModifyDocument()` is same as just calling
`EditorBase::EnsureNoPaddingBRElementForEmptyEditor()` and
`EditorBase::MaybeCreatePaddingBRElementForEmptyEditor()` so that this patch
gets rid of the method, then, creates `HTMLEditor::OnModifyDocumentInternal()`
and makes it do same thing.
Differential Revision: https://phabricator.services.mozilla.com/D41161
--HG--
extra : moz-landing-system : lando
`HTMLEditRules::WillLoadHTML()` does exactly same thing as
`EditorBase::EnsureNoPaddingBRElementForEmptyEditor()`. Therefore, we can
get rid of it and make `HTMLEditor::LoadHTML()` not use `HTMLEditRules`.
Differential Revision: https://phabricator.services.mozilla.com/D41160
--HG--
extra : moz-landing-system : lando
`TextEditRules::CreatePaddingBRElementForEmptyEditorIfNeeded()` is used by
both `TextEditor` and `HTMLEditor` so that this moves it into `EditorBase`.
Additionally, making `TextEditor::MaybeChangePaddingBRElementForEmptyEditor()`
call it if there is no content. Then, we can avoid the dependency of them.
Differential Revision: https://phabricator.services.mozilla.com/D41159
--HG--
extra : moz-landing-system : lando
`TextEditRules::RemoveRedundantTrailingBR()` is used only by multiline text
editor (i.e., `<textarea>`). Therefore, it should be moved into `TextEditor`.
Differential Revision: https://phabricator.services.mozilla.com/D41158
--HG--
extra : moz-landing-system : lando
`TextEditRules::WillUndo()` and `TextEditRules::WillRedo()` only check whether
the editor is readonly/disabled or not. So, `TextEditor::UndoAsAction()` and
`TextEditor::RedoAsAction()` should do it first.
`TextEditRules::DidUndo()` and `TextEditRules::DidRedo()` only set or unset
`mPaddingBRElementForEmptyEditor` if it's restored by undo or redo.
Therefore, we can move the code into `TextEditor::UndoAsAction()` and
`TextEditor::RedoAsAction()`.
Note that this patch makes `TextEditor::UndoAsAction()` discard the result of
`TransactionManager::Undo()` because this is inconsistent from what
`TextEditor::RedoAsAction()` does and this was changed by part 5 of bug 1447924.
https://hg.mozilla.org/mozilla-central/rev/869a1445816be7f43f54f7c97f28e4c6273fa75f
Differential Revision: https://phabricator.services.mozilla.com/D41157
--HG--
extra : moz-landing-system : lando
`TextEditRules::WillInsert()` is not used with initial purpose since
`HTMLEditor` always works with `HTMLEditRules` and its `WillDoAction()`
always handles `EditSubAction::eInsertElement`.
Additionally, its name is too generic since it does non-related 3 things.
One is checking whether the editor is readonly or disabled. However, this
may not be necessary since its callers may have already checked it or
just ignored the result. So, this should be check by each caller.
Next one is masking password if auto-masking is enabled. This is `TextEditor`
specific feature so that this patch moves the code into
`TextEditor::MaybeDoAutoPasswordMasking()`.
Final one is removing empty `<br>` element for empty editor if there is.
This is common feature so that this patch moves this code into
`EditorBase::EnsureNoPaddingBRElementForEmptyEditor()`.
Differential Revision: https://phabricator.services.mozilla.com/D41156
--HG--
extra : moz-landing-system : lando
`TextEditRules::mPaddingBRElementForEmptyEditor` are used by both `TextEditor`
and `HTMLEditor`. Therefore, it should be in `EditorBase`.
This patch makes `TextEditRules` and `HTMLEditRules` directly access the
private member of `EditorBase` temporarily. It'll be fixed by the following
patches.
Differential Revision: https://phabricator.services.mozilla.com/D41155
--HG--
extra : moz-landing-system : lando
