The `AutoTrackDOMPoint` constructor which takes `nsCOMPtr<nsINode>&` and
`uint32_t` is not used anymore. Thus, we can get rid of the mode.
And `FlushAndStopTracking()` should be replaced with
`Flush(StopTracking::Yes)` and align the relatives to use the same
API style.
Differential Revision: https://phabricator.services.mozilla.com/D283634
`IMEContentObserver` has a cache to avoid to compute start offset of
inserted or removing text every time. However, that works only when
the same mutation occurs of consecutive nodes.
`MoveSiblingsTransaction` uses `nsINode::InsertBefore()` directly to
connected nodes. Therefore, from `IMEContentObserver` point of view,
multiple pairs of removing text and inserting text occur. So,
`IMEContentObserver` cannot use its cache.
Therefore, `MoveSiblingsTransaction` should remove all nodes first.
Then, `IMEContentObserver` can use the cache for deleting text.
Finally, `MoveSiblingsTransaction` should insert all removed nodes to
the DOM. Then, `IMEContentObserver` can use the cache for inserting
text.
Differential Revision: https://phabricator.services.mozilla.com/D259720
`AutoRangeArray` is created for making some handlers of the editor classes free
from `Selection` and `nsFrameSelection` while handling the edit actions.
However, the method still depends on `nsFrameSelection` instance since its
callees are instance methods of `nsFrameSelection`.
However, `EditContext` requires completely free methods to compute target
ranges of `beforeinput`. Therefore, we need to make it not depend on
`Selection` nor `nsFrameSelection`.
The common method, `nsFrameSelection::CreateRangeExtendedToSomewhere`, requires
`PresShell`, selection limiter which is set only when the selection is an
independent selection like in a selection for a text control, selection
ancestor limiter which is set only when an editing host has focus, caret
association hint to put caret to end of preceding line or start of following
line if selection range is collapsed at a line break and caret bidi level for
considering caret position around line break in bidi text. They are now stored
by `nsFrameSelection` and modified when selection range is changed in some
cases. Basically, the method is called without updating its ranges and if and
only if it's initialized with `Selection`. So, simply caching the
`nsFrameSelection`'s values solves the issues in the most cases, but this
patch makes `AutoRangeArray` adjust the value only when its `Collapse` is
called because `Selection` automatically updates it and we can compute the
value without `Selection` nor `nsFrameSelection`.
After applying this patch, `AutoRangeArray` has two meanings, one is the
instance is a proxy for `Selection`. The other is a container for a range to
call methods which take pointer or reference to it. To make this differences
checked at build time, this patch creates a new subclass of it, and renamed to
`AutoClonedRangeArray` to make it clearer that what are stored in the array.
Oddly, `AutoRangeArray(nsRange&)` constructor has not been cloned. Therefore,
I make it and its subclass version clone before storing into the array. Then,
one caller needs to change which range should be tracked.
Differential Revision: https://phabricator.services.mozilla.com/D232174
`HTMLEditor` and its helper classes need to handle both a `<br>` and a
preformatted linefeed in the same way. However, the former is an independent
node but the latter is a char in a `Text` node. Therefore, it makes the
implementation simpler for making each `<br>` element handler treat a
preformatted linefeed in same path if there is a class to store the both
types of a line break.
Differential Revision: https://phabricator.services.mozilla.com/D231654
`AutoBlockElementsJoiner::HandleDeleteNonCollapsedRange` may delete multiple
lines if selection ranges partially select table(s) because in such case, we
keep the table structure but delete the selected content. Therefore, it
requires clean up padding line breaks in multiple places. However, when it
wants to clean them up, there is no enough information about the deleted ranges.
Therefore, this patch makes the one of the callee return it.
Depends on D229857
Differential Revision: https://phabricator.services.mozilla.com/D229858
`MoveNodeResult` is usually merged with an ignored result which store the first
insertion point and multiple handled results which store next insertion points.
Therefore, the final result can store the moved content nodes range. The range
is useful to clean up unnecessary line breaks and to insert padding line breaks.
Therefore, this patch makes `MoveNodeResult` compute the range in `|=` operator
and make the methods return the range of moved content.
Differential Revision: https://phabricator.services.mozilla.com/D229857
The root cause of this bug is, we tried to compute new offset with offset at
the new node. However, as explained in the inline comments, it should compute
the offset with the right node offset in the new mode. Therefore, it needs
to handle it by itself instead of just calling `SelAdjInsertNode`.
Differential Revision: https://phabricator.services.mozilla.com/D171965
As of the prior patch, these are no longer needed. I removed
these with a script, then ran clang-format on the files, then
manually reverted a few unrelated changed from the formatter.
Differential Revision: https://phabricator.services.mozilla.com/D164829
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
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
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
When selection is `abc<b>[def</b>]ghi`, `insertParagraph` command will delete
the `<b>` element first, then, `Selection` becomes `abc{}ghi`. Then,
`HTMLEditor::InsertParagraphSeparatorAsSubAction` wraps all of the line in
the default paragraph, `<div>`, with
`HTMLEditor::FormatBlockContainerWithTransaction` (although this is incompatible
behavior with the other browsers). At this time, new `<div>` is inserted before
the first text node and then, move the text nodes into the new `<div>`.
However, `RangeUpdater::DidMoveNode` just slides the offsets if containers of
registered DOM points are the ex-parent of the moving nodes. Therefore, the
tracked selection range in `HTMLEditor::FormatBlockContainerWithTransaction`
become `<div></div>abc{}def`, then, `<div>abcdef</div>{}`, but the expected
behavior is of course, `<div>abc{}def</div>`, then, split the new `<div>`.
So the problem is, `DidMoveNode` assumes that DOM points won't point the moving
content node. If the node is pointed, it should keep pointing in the new
parent.
Note that the expectations of new tests are based on Chrome, therefore, the
new known failures are incompatible with Chrome.
Differential Revision: https://phabricator.services.mozilla.com/D156798
In bug 1739524, I misunderstood the meaning of `aOffset` of `SelAdjJoinNodes`.
After joining 2 nodes, and a point points right node which will have ex-left
node content, the point needs to point ex-start of the right node to keep
next insertion point as-is. Therefore, it's not useful with new join nodes
direction, it needs to know the ex-offset of the right node.
Differential Revision: https://phabricator.services.mozilla.com/D155438
This patch makes it take `AutoRangeArray` and it can save a snapshot of the
ranges and restore it like `AutoSelectionRestorer`.
For accessing `HTMLEditor::RangeUpdaterRef` for clearing the saved ranges,
`AutoRangeArray` needs to store `HTMLEditor`. Therefore, its constructors
and the destructor cannot be inlined in the header due to the include-hell.
Differential Revision: https://phabricator.services.mozilla.com/D149089
Creating both `DeleteNodeTransaction` and `InsertNodeTransaction` wastes
memory. They should be done in an instance instead.
Fortunately, no edit action listener checks whether the deleted node is still
in the composed document or not, etc. Therefore, we can simply notify them of
both deletion and insertion which were done in
`EditorBase::InsertNodeWithTransaction` and
`EditorBase::DeleteNodeWithTransaction`. Note that previously, the range
updater needs to ignore the notifications from them while the node is being
moved. However, it does not require anymore. Therefore, this patch makes it
stop locking, and that would fix minor problem in the case of legacy mutation
event listeners run another edit action.
On the other hand, this changes some edge cases handling of
`MoveNodeWithTransaction` which are detected by the WPT. According to the
previous result of applying this patch, `nsINode::InsertBefore` fails and that
leads some errors at updating the changed range. I guess that the cause is
that there is some bugs at updating insertion point after deleting the node from
the DOM tree around here:
https://searchfox.org/mozilla-central/rev/0ffae75b690219858e5a45a39f8759a8aee7b9a2/editor/libeditor/HTMLEditor.cpp#5058-5071
However, it's safely fixed by the new code which does not remove the node from
the DOM tree explicitly. So, I think that it's safe to accept this behavior
change for web apps in the wild.
Differential Revision: https://phabricator.services.mozilla.com/D146397
The creation and initialization cost of `nsRange` is expensive. And just
comparing DOM points does not require the checks of ancestors which are
required by `nsRange` to observe DOM mutations. Therefore, although the
logic is not exactly same, we can skip the additional unnecessary cost.
Note that the Editor module checks without `nsRange`'s strict initialization
in most places so that it must not be required to do the strict check only in
`SelectionState`.
Additionally, this moves some definitions of `SelectionState` methods to
its header for making them inlined since the dependency is now reduced.
Finally, some methods are renamed to align to `Selection` API.
Depends on D145226
Differential Revision: https://phabricator.services.mozilla.com/D145227
This is the most cases of the warnings. If previous transactions occurred in
now disconnected subtree, e.g., they are replaced by setting `innerHTML` to
check different scenarios, `nsRange::CompareBoundaryPoints` return error.
Therefore, this case should be handled before comparing the range boundaries
and logged for making easier to debug.
Depends on D145225
Differential Revision: https://phabricator.services.mozilla.com/D145226
The warnings are caused by comparing a range with non-positioned range, or
comparing non-positioned ranges. And `SelectionState::IsCollapsed()` may
return `true` when it has and only has a non-positioned range.
Therefore, this patch adds new method which checks whether the selection cache
points a caret position or not instead of the "collapsed" state, and make
`PlaceholderTransaction::StartSelectionEquals` check is result before calling
`SelectionState::Equals`.
Differential Revision: https://phabricator.services.mozilla.com/D145225
It's an internal API corresponding to `Selection.getRangeAt` DOM API.
I think that it should use `uint32_t` rather than `size_t` because of the
consistency with the DOM API and `Selection::RangeCount()`.
This patch fixes all callers of `GetRangeAt()`, and rewrites it with ranged-
loops unless original ones do not refer `RangeCount()` every time and may run
script in the loop.
Differential Revision: https://phabricator.services.mozilla.com/D128848
Currently, it assumes that `aLeftNode` is new node and the other is the original
one. However, we need the opposite case when we fix bug 1735608.
Unfortunately, we cannot test the new path until we fix bug 1735608. Therefore,
the new path may contain some bugs.
Differential Revision: https://phabricator.services.mozilla.com/D130427
Currently, it assumes that all children or text data of `aRightContent` is
moved into `aLeftContent`. However, we need the opposite case when we fix
bug 1735608.
Unfortunately, we cannot test the new path until we fix bug 1735608. Therefore,
the new path may contain some bugs.
Differential Revision: https://phabricator.services.mozilla.com/D130426
The crash is caused by the infinite loop to delete unnecessary ranges.
The `i` is always equals or larger than `0` due to unsigned. So, if
`Selection::RangeCount()` is `0`, `i` can be `UINT32_MAX`.
Instead, we can use `nsTArray<RefPtr<RangeItem>>::TruncateLength()`.
Differential Revision: https://phabricator.services.mozilla.com/D119564
It should be treated as `uint32_t` since DOM API does so. However, there are
some exceptions:
* Result of `nsINode::ComputeIndexOf()`
* Result of `nsAString` methods
They return `-1` as not found, and anyway, they cannot treat large integer
than `INT32_MAX`. Therefore, this patch does not touch around them.
Differential Revision: https://phabricator.services.mozilla.com/D118933
Currently, when `HTMLEditor` replaces text in a text node, `HTMLEditor`
creates a set of `DeleteTextTransaction` and `InsertTextTransaction`.
However, this has bad impact for footprint and causes the callers messy.
This patch creates `ReplaceTextTransaction` instead and
`HTMLEditor::ReplaceTextWithTransaction()` as its wrapper. Unfortunately,
this becomes not calling `nsIEditActionListener::DidDeleteText()`, however,
this is not used by mozilla-central, comm-central nor BlueGriffon. IIRC,
it was not removed for some legacy addons of Thunderbird. Therefore, it
must be okay to remove it.
Differential Revision: https://phabricator.services.mozilla.com/D76078
`nsRange` instances are allocated a lot in the heap especially by editor and
spellchecker. The allocation cost is too bad for benchmarks. Therefore,
we should reuse released instances as far as possible. For managing it in
static factory methods of `nsRange`, we need to hide `nsRange` constructor.
Differential Revision: https://phabricator.services.mozilla.com/D61237
SelectionState doesn't save and restore current selection's direction. So the
direction of selection is always default after undoing.
So we should restore direction of selection.
Differential Revision: https://phabricator.services.mozilla.com/D2259
To reduce QI, I would like to replace nsIDOMNode with nsINode. And some
parameters in *DataTransder.cpp's methods is unused (it uses as nullptr),
so we should remove these parameters.
Also nsIDOMNodeList is unused now, so we should remove this including.
MozReview-Commit-ID: 1QTIkxDazjJ
A lot of methods take |const EditorRawDOMPoint&| as their argument. However,
some of them are called with EditorDOMPoint::AsRaw(). This is not good for
performance because:
1. Needs to create temporary instance of EditorRawDOMPoint.
2. EditorRawDOMPoint::AsRaw() may be used by simple mistake.
3. Such methods may call EditorRawDOMPoint::Offset(), however, it's not copied
to the original EditorDOMPoint instance. So, callers may need to compute
offset again.
So, such methods should be changed to template methods if they are not virtual
method and AsRaw() should be gotten rid of for prevent it looking not expensive.
MozReview-Commit-ID: DAqqW4a2EMS
Daniel Holbert