Native behaviour on MacOS dictates one whitespace being removed after double-clicking a word and pressing delete.
This behaviour is achieved by saving the information that the selection is created by doubleclick to the `nsFrameSelection`
and using it in the `DeleteRangeTransaction`, where the range is extended by one whitespace character before or after the range.
Differential Revision: https://phabricator.services.mozilla.com/D159613
Spellchecker itself is using idle callback for certain things, so this is just trying to mimic that.
But purely guess work. The test is inherently racy since nothing guarantees the OS gives cpu time for the parent side.
We could add some notification that spellchecking is done, but not sure how a reftest coudl then access that notification -
but that would require some changes to the editor/spellchecker side too.
Differential Revision: https://phabricator.services.mozilla.com/D160022
The testcase is tricky. It creates 2 `Selection` ranges, one is collapsed at
end of the `<html>`, the other is collapsed in the new `<h3>` element. The
first one is ignored by delete handler since Gecko does not allow to edit
outside `<body>` for now.
Then, deleting non-collapsed selection ranges tries to delete empty parent
blocks at the remaining collapsed selection range in the `<h3>`. At this time,
it works with `nsIEditor::eNone`. Then, its `GetNewCaretPosition` does not
return a valid point. Then, the `Run` does not remove the `Selection` range
outside the `<body>`.
Therefore, `HTMLEditor::DeleteSelectionAndPrepareToCreateNode()` will see
2 ranges, then, hit the assertion.
Although there are some other cases which meet 2 or more `Selection` ranges
after deletion in `DeleteSelectionAndPrepareToCreateNode`, but for now, we
should make `AutoEmptyBlockAncestorDeleter::Run` collapse `Selection` when
it deletes empty ancestors.
Differential Revision: https://phabricator.services.mozilla.com/D159581
The failure on Android is caused by async initialization of `<textarea>` runs
after initializing selection in it. Therefore, `ArrowUp` is synthesized when
all text is selected and collapses it to start.
Then, I see another failure on Linux and macOS in `contenteditable` case at
running TV and TV-nonfis. It seems that it's caused by remaining `Selection`
of the preceding test (`vertical-rl` case) does something wrong. Once
removing all ranges at `blur`ring, the failures in `vertical-lr` case has
gone.
Differential Revision: https://phabricator.services.mozilla.com/D159566
Including it in `HTMLEditor.h` causes creating static constructors for
the modules which include `HTMLEditor.h`. And the public methods are used
only in the `libeditor`. Therefore, we can make `HTMLEditor.h` stop including
it and stop exposing `JoinSplitNodeDirection.h`.
Differential Revision: https://phabricator.services.mozilla.com/D159363
One of the reason of bug 1793873 is, `HTMLEditUtils::IsSplittableNode` returns
`false` for `<textarea>`. Then, `insertParagraph` etc command with `Selection`
collapsed in `<textarea>` causes splitting the `<textarea>` and it's not split
at same parent.
Then, I found that we need to take care `<button>` specially. It creates a
block formatting context, and it's a form control so that it should never be
split. Therefore, `HTMLEditor::GetAncestorElement` requires to take care of
`<button>` element with additional scan option for minimizing the risk.
Differential Revision: https://phabricator.services.mozilla.com/D159229
It restores `Selection` with `AutoSelectionRestorer` instance created first.
Therefore it does not want the callers (currently, only
`HTMLEditor::HandleOutdentAtSelection` only) change `Selection` after doing it
without special reasons. Therefore, it shouldn't return last caret point
suggestion which is not a good point to put caret actually. Then, callers
do not need to handle it as they've never done.
Differential Revision: https://phabricator.services.mozilla.com/D159231
nsISupports.h includes nsISupportsBase.h, so it should be equivalent.
In the next patch, I'm changing things so that nsISupports is defined in
nsISupports.h instead of nsISupportsBase.h, and deleting the latter, so
this change will be needed anyways. I'm guessing people were using IWYU
or something like that.
Differential Revision: https://phabricator.services.mozilla.com/D159169
Between splitting a node and undoing it, web apps can move split nodes anywhere.
Therefore, it shouldn't assume they are always in same parent node, and
`RangeUpdater::SelAdjJoinNodes` needs to handle it correctly.
Unfortunately, `RangeUpdater::SelAdjJoinNodes` cannot handle nested cases
correctly, e.g., right node was in `aRemovedContent` or right node was in
the container of `aStartOfRightContent.GetContainer()`. However, it's not
a new regression, and such complicated situation breaks undoing anyway.
Therefore, I think that we don't need to care about it for now.
Differential Revision: https://phabricator.services.mozilla.com/D159230
It's the only method which is used outside of the editor module. Therefore,
we can stop exposing `EditorUtils.h` later.
Depends on D158634
Differential Revision: https://phabricator.services.mozilla.com/D158635
It's currently referred from outside only by `TextServicesDocument.cpp`.
It's not under `libeditor` but in the editor module. Therefore, let's allow
it to refer the header files under `libeditor` directly. Then, we can stop
exposing `HTMLEditUtils.h`.
Depends on D158633
Differential Revision: https://phabricator.services.mozilla.com/D158634
They are used in public inline methods of `HTMLEditor`. Therefore, they should
be defined in an exposed header file without other internal use things for the
editor module. Then, we can stop exposing `HTMLEditHelpers.h`.
Depends on D158632
Differential Revision: https://phabricator.services.mozilla.com/D158633
It's always called with `DeleteSelectedContent::Yes`. Therefore, the argument
is not required. Then, its name can be `InsertFromTransferableAtSelection`.
Differential Revision: https://phabricator.services.mozilla.com/D158483
It just creates `HTMLWithContextInserter` and calls its `Run()`. Then, `Run()`
works as a sub action handler. Therefore, we can make `DoInsertHTMLWithContext`
(which sounds like a helper class of a handler method) a sub action handler.
Then, it can handle the optional insertion point in it and
`HTMLWithContextInserter::Run` does not need to handle it.
Differential Revision: https://phabricator.services.mozilla.com/D158482
They use `bool` arguments a lot. Therefore, some call-sites are hard to read.
They should be replaced with `enum class`es. Note that this patch does not
make the raw value of new `enum class`es to `bool` unless they are used in the
heap.
Differential Revision: https://phabricator.services.mozilla.com/D158481
`nsIEditor.undo` and `nsIEditor.redo` are called with `1` except by the search
bar, and search bar wants to undo everything to reset the value. Therefore,
search bar needs an API to undo all, and the others do not need the number of
undoing/redoing transactions. Therefore, this patch adds `nsIEditor.undoAll`
for search bar, and remove the arguments from `nsIEditor.undo` and
`nsIEditor.redo`.
Differential Revision: https://phabricator.services.mozilla.com/D158338
They are methods to take 2 out params, and it's not convenient for light use.
I think that there should be 3 attributes, `undoRedoEnabled`, `canUndo` and
`canRedo`. Then, the findbar does not need to check number of transactions
with `nsITransactionManager`.
Differential Revision: https://phabricator.services.mozilla.com/D158337
`nsIEditor.transactionManager` is used only for some simple purposes, however,
`nsIEditor` exposes the rich API. That makes it harder to maintain internal
code around transactions. Instead, `nsIEditor` exposes only simple and
necessary APIs.
This patch creates a new API to clear undo/redo history and make the users in
mozilla-central use it instead of using `nsITransactionManager.clear()`.
Differential Revision: https://phabricator.services.mozilla.com/D158336
I misunderstood the traditional behavior before bug 1789967, that is,
`HTMLEditor` collapsed `Selection` to end of the deepest last child container
element **or text node** of the `<body>`, rather than to end of **only** the
deepest last child container element. This misunderstanding caused the backout
of the first landing, but I didn't realize this mistake.
Therefore, this patch makes the crashtests which are touched in bug 1789967 and
the collapsed point is at a text node (or failed to consider the collapsed
point) collapse `Selection` to end of the deepest last text node of their
`<body>` elements.
Differential Revision: https://phabricator.services.mozilla.com/D158177
If splitting element is a list item element,
`HandleInsertParagraphInListItemElement` may just move it into a list element
if it's followed by the list element, or may create new paragraph element if
the list item element is empty and last list item element.
If splitting element is a heading element,
`HandleInsertParagraphInHeadingElement` may create new paragraph element if
splitting at end of the heading element and not followed by `<br>`.
Therefore, neither `SplitNodeResult` nor `CreateElementResult` is proper for
their result type, and the caller `InsertParagraphSeparatorAsSubAction`, just
wants to know the right handle element which should contain caret and a caret
point suggestion.
For solving these issues, this gives an alias to `CreateElementResult`, makes
them return the right paragraph even if it's not newly created nor split
element, and replaces `blockElementToPutCaret` with the returned element.
Differential Revision: https://phabricator.services.mozilla.com/D158106
The test compares the node name of the split element and its previous sibling
element, so it should switch the scanning direction with the pref to switch
split node direction.
Note that the behavior is different from the other browsers in some cases.
Therefore, we cannot port this to WPT.
Differential Revision: https://phabricator.services.mozilla.com/D158105
The check assumes that the container of `aPointToInsert` is always right node.
However, its child is moved to new node if we switch the direction, so we should
make it check the edge case with `SplitNodeResult::GetNextContent()`.
Differential Revision: https://phabricator.services.mozilla.com/D158104
The reason why the changes of `SplitNodeTransaction` is smaller than
`JoinNodesTransaction` is, `HTMLEditor::DoSplitNode` does almost all things
instead.
Differential Revision: https://phabricator.services.mozilla.com/D158102
This patch adds new path to adjust selection for the new split node direction.
So this does not change any behavior of the existing path.
Differential Revision: https://phabricator.services.mozilla.com/D158100
This changes the existing path's behavior a bit. However, it should not affect
to web apps in the wild because it must be really rare case that web apps
inserting new nodes into the removing node while moving its children.
Differential Revision: https://phabricator.services.mozilla.com/D158099
This patch add new path to adjust selection for the new join nodes direction.
So this does not change any behavior of the existing path.
Differential Revision: https://phabricator.services.mozilla.com/D158098
Jan-Niklas Jaeschke
Olli Pettay
Masayuki Nakano
Tom Schuster
Sandor Molnar
Andrew McCreight
Cristian Tuns
Chengyu Sun