If I run `./mach wpt testing/web-platform/tests/editing` with just enabling the
new normalizer, I see some assertion failures which are caused by scanning or
updating non-editable regions.
According to the debug result, it's caused by sometimes caret point is suggested
in non-editable node and further handling will use various utilities which
asserts whether the target is editable.
Ideally, we should add assertion into `CaretPoint`, but it's hard and I don't
have enough time to fix every regression which will be found by fuzzing tests
even though most of them are not so important in the real web apps. Therefore,
this fixes only the causes which I found in the WPTs.
Differential Revision: https://phabricator.services.mozilla.com/D238408
First, `EditorBase::CreateTransactionForDeleteSelection` returns an instance of
`EditAggregateTransaction`. It's a base class of `PlaceholderTransaction` and
`DeleteRangeTransaction` but it's also a concrete class. However, it's too
generic. Therefore, this patch creates `DeleteMultipleRangesTransaction` which
is a simple sub-class of it, and makes `EditAggregateTransaction` be an abstract
class. Then, add `AddChild` methods to each concrete class to restrict the
type of child transactions.
Next, `DeleteRangeTransaction` contains only `DeleteNodeTransaction` and
`DeleteTextTransaction`. Therefore, once they have a common base class,
we can check the type easier. Therefore, this patch also adds
`DeleteContentTransactionBase` and
`EditorBase::CreateTransactionForCollapsedRange` becomes clearer what it
returns.
With these changes, `DeleteRangeTransaction` obviously contains only
`DeleteContentTransactionBase`, `DeleteMultipleRangesTransaction` contains only
`DeleteRangeTransaction`, `DeleteNodeTransaction` and `DeleteTextTransaction`.
And they are guaranteed at build time (at least from outside the classes).
***
fix
Differential Revision: https://phabricator.services.mozilla.com/D169038
If the range ends at a node, its parents are never iterated by
`ContentSubtreeIterator` because it collects the nodes end in the given range.
However, if the range starts from a new empty parent node, e.g.,
```
abc[<b>def]</b>
```
Then, the `<b>` element should be deleted rather than alive as `abc<b></b>`.
Therefore, unless the end container does not match with start container, it
should extend the range to contain empty parents while the range ends at end
of a node.
Differential Revision: https://phabricator.services.mozilla.com/D160165
If the range ends at a node, its parents are never iterated by
`ContentSubtreeIterator` because it collects the nodes end in the given range.
However, if the range starts from a new empty parent node, e.g.,
```
abc[<b>def]</b>
```
Then, the `<b>` element should be deleted rather than alive as `abc<b></b>`.
Therefore, unless the end container does not match with start container, it
should extend the range to contain empty parents while the range ends at end
of a node.
Differential Revision: https://phabricator.services.mozilla.com/D160165
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
It does different thing for `TextEditor` and `HTMLEditor`, and used almost
internally. Therefore, it should be implemented in the sub classes and
we should name them better.
Differential Revision: https://phabricator.services.mozilla.com/D157407
It does different thing for `TextEditor` and `HTMLEditor`, and used almost
internally. Therefore, it should be implemented in the sub classes and
we should name them better.
Differential Revision: https://phabricator.services.mozilla.com/D157407
First, move methods of `HTMLEditor` which collapse `Selection` to `EditorBase`.
Then, make editor stop accessing `Selection::CollapseInLimiter` directly.
Differential Revision: https://phabricator.services.mozilla.com/D143814
Our internal code can touch internal `Selection` object of `<input>` and
`<textarea>`, thus, selection ranges in them may be not in the text node.
Therefore, we should make `TextEditor` instance try to shrink the deleting
ranges into the text node first.
In a followup bug (perhaps, bug 1734846), we should split the deletion code into
`TextEditor` and `HTMLEditor` for making the code simpler and never regressed.
Differential Revision: https://phabricator.services.mozilla.com/D133181
I just missed some callers of it. And I also make
`EditorBase::DeleteNodeTransaction()` a virtual method and `HTMLEditor` override
it. Then, even if it's called by helper classes, the `HTMLEditor` version is
always called.
Differential Revision: https://phabricator.services.mozilla.com/D123578
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
CLOSED TREE
Backed out changeset d3d67293f115 (bug 1623413)
Backed out changeset 75ed1b8a5c67 (bug 1623413)
Backed out changeset 0eef32d6a454 (bug 1623413)
In the next patch, we need to update unmasked range when anonymous text node
in `<input type="password">` is modified. This patch makes editor manage it
easier to update the range.
Note that the next patch also handles text node creation and destruction.
Differential Revision: https://phabricator.services.mozilla.com/D38003
This patch makes ContentIteratorBase, PostContentIterator, PreContentIterator
and ContentSubtreeIterator classes non-refcountable because most users can
create their instances in stack and such users may be in a hot path. So,
we can save a lot of cost of instantiation.
Unfortunately, only ScriptableContentIterator creates one of the concrete
classes and needs to destroy it properly. Therefore, its
EnsureContentIterator(), destructor, traverse and unlink code becomes messy.
However, ScriptableContentIterator was designed for automated tests and we
need to maintain it not so many times. Therefore, improvement of other
users must be worthwhiler than this demerit.
Differential Revision: https://phabricator.services.mozilla.com/D15928
Now, all users of ContentSubtreeIterator can access it directly. This patch
makes them use the concrete class directly.
Differential Revision: https://phabricator.services.mozilla.com/D15920
This patch makes ContentIteratorBase, PostContentIterator, PreContentIterator
and ContentSubtreeIterator classes non-refcountable because most users can
create their instances in stack and such users may be in a hot path. So,
we can save a lot of cost of instantiation.
Unfortunately, only ScriptableContentIterator creates one of the concrete
classes and needs to destroy it properly. Therefore, its
EnsureContentIterator(), destructor, traverse and unlink code becomes messy.
However, ScriptableContentIterator was designed for automated tests and we
need to maintain it not so many times. Therefore, improvement of other
users must be worthwhiler than this demerit.
Differential Revision: https://phabricator.services.mozilla.com/D15928
Now, all users of ContentSubtreeIterator can access it directly. This patch
makes them use the concrete class directly.
Differential Revision: https://phabricator.services.mozilla.com/D15920
nsIEditor::ShouldTxnSetSelection() is used only by DeleteRangeTransaction
(even if including comm-central and BlueGriffon) and there is a non-virtual
method EditorBase::GetShouldTxnSetSelection(). So, we can remove this.
MozReview-Commit-ID: JWSCw9k6lI0
Although, we don't need factory methods for DeleteRangeTransaction,
EditAggregateTransaction nor PlaceholderTransaction, for consistency with the
other transaction classes, they should have factory methods for making easier
to write the code.
For not making the performance slow down, they should be inline methods.
MozReview-Commit-ID: 7jl5yZNFYmP
EditorBaseTransaction::CreateTxnForDeleteNode() just hides what it does.
Instead, let's create a factory method, DeleteNodeTransaction::MaybeCreate()
for making callers clearer.
MozReview-Commit-ID: 8WUYN0BjKSU
DeleteTextTransaction should have 3 factory methods. One is, simply to create
an instance with a pair of offset and length. The others are, to create an
instance for deleting a previous or next character at offset.
The former was EditorBase::CreateTxnForDeleteText() and the latter was
EditorBase::CreateTxnForDeleteCharacter(), but this patch creates
DeleteTextTransaction::MaybeCreate() for the former,
DeleteTextTransaction::MaybeCreateForPreviousCharacter() and
DeleteTextTransaction::MaybeCreateForNextCharacter() for the latter.
MozReview-Commit-ID: DFELbmAJDo3
By the fix of bug 1407352, DeleteRangeTransaction::CreateTxnsToDeleteBetween()
starts to use child node instead of a set of container node and offset in it.
However, this is ugly and using RawRangeBoundary may reduce computing offset
more. So, they should use RawRangeBoundary to refer delete points.
MozReview-Commit-ID: pwDHOxpz0E
This does NOT change variable names like |endNode| because it's not odd and somebody use it for nsINode and endContent for nsIContent. So, changing them needs more work.
MozReview-Commit-ID: 22imUltlu5R
This does NOT change variable names like |startNode| because it's not odd and somebody use it for nsINode and startContent for nsIContent. So, changing them needs more work.
MozReview-Commit-ID: H19pTDprRuT
Sylvestre Ledru