Although it does not need to join text nodes around the `<br>` element since
its previous node is `<hr>`, it can use
`WhiteSpaceVisibilityKeeper::DeleteContentNodeAndJoinTextNodesAroundIt()` too.
Depends on D87029
Differential Revision: https://phabricator.services.mozilla.com/D87030
It works with the traditional white-space normalizer. Therefore, it should
be moved into `WhiteSpaceVisibilityKeeper`.
Depends on D86910
Differential Revision: https://phabricator.services.mozilla.com/D87029
This patch changes the behavior in the following 2 points:
* When both content is in same block element, won't return error
* When `<hr>` element has block children accidentally, this solves its container
The former case must not change actual behavior because
`AutoBlockElementsJoiner` is used with content nodes which are in different
blocks.
Differential Revision: https://phabricator.services.mozilla.com/D86886
Now all users of the method is methods of `AutoBlockElementsJoiner`.
Therefore, we can move it into the nested class. And for splitting the
method in the following patch, it should be a nested class of
`AutoBlockElementJoiner`.
Depends on D86791
Differential Revision: https://phabricator.services.mozilla.com/D86881
And now, `HTMLEditor::JoinNodesDeepWithTransaction()` is used only by
`AutoBlockElementsJoiner`. Therefore, this patch moves it to the
stack only class.
Differential Revision: https://phabricator.services.mozilla.com/D86789
For making the following review easier, this patch just moves some part of
`HandleDeleteNonCollapsedRanges()` into `AutoBlockElementJoiner`.
Differential Revision: https://phabricator.services.mozilla.com/D86786
Now, only when deleting table cell contents, `HandleDeleteSelectionInternal()`
depends on `Selection`. However, this can be moved to `HandleDeleteSelection()`
because recursive callers expects `Selection` is collapsed by its previous job.
Differential Revision: https://phabricator.services.mozilla.com/D86183
This patch moves `EditorBase::ExtendSelectionForDelete()` into `AutoRangeArray`
and make it stop modifying `Selection`.
The method extends anchor-focus range with `nsFrameSelection` and it refers
bidi information in it too. Therefore, it needs to be called before modifying
anchor-focus range of `Selection`. Unfortunately, this makes the code messy,
but for now, we should take this. In the future, we should make the API of
`nsFrameSelection` free from `Selection`.
Differential Revision: https://phabricator.services.mozilla.com/D86182
Creation cost of `nsRange` is expensive, and it's only user,
`HTMLEditor::HandleDeleteNonCollapsedRanges()` wants to extend the range.
Therefore, we can make it directly extend the given range.
Differential Revision: https://phabricator.services.mozilla.com/D85849
It may be faster to use `AutoTrackDOMRange` directly. Therefore, current
`WhiteSpaceVisibilityKeeper::PrepareToDeleteRange()` should be renamed to
`WhiteSpaceVisibilityKeeper::PrepareToDeleteRangeAndTrackPoints()` and
we should make `HTMLEditor::HandleDeleteNonCollapsedSelection()` track
the range by itself.
Depends on D85846
Differential Revision: https://phabricator.services.mozilla.com/D85847
There is only `AutoTrackDOMPoint`, but
`HTMLEditor::HandleDeleteNonCollapsedSelection()` requires to tack pairs of
DOM points, i.e., requires to track ranges. For making the code simpler,
we should have range tracker too.
Depends on D85845
Differential Revision: https://phabricator.services.mozilla.com/D85846
This patch makes `EditorBase::DeleteSelectionWithTransaction()` a wrapper of
`EditorBase::DeleteRangesWithTransaction()`.
Differential Revision: https://phabricator.services.mozilla.com/D85686
Currently, `nsIEditActionListener::WillDeleteSelection()` notifies
`FinderHighliter` of deleting selection ranges. But it's referred only
the ranges, not selection object itself and editor shouldn't modify
`Selection` as far as possible for reducing the runtime cost.
Therefore, it should be replaced with this new API. Then,
`EditorBase::WillDeleteSelectionWithTransaction()` can be rewritten as
`EditorBase::WillDeleteRangesWithTransaction()` later.
Differential Revision: https://phabricator.services.mozilla.com/D85681
The recursive call is not safe. We should do it more carefully. For now,
we should not hide the recursive call into the leaf handler.
Differential Revision: https://phabricator.services.mozilla.com/D85573
This patch creates new stack only class, `HTMLEditor::AutoBlockElementsJoiner`
and splits `HTMLEditor::HandleDeleteCollapsedSelectionAtCurrentBlockBoundary()`
to considering the content nodes to be joined part and doing join the nodes
part.
Differential Revision: https://phabricator.services.mozilla.com/D85567
This patch creates new stack only class, `HTMLEditor::AutoBlockElementsJoiner`
and splits `HTMLEditor::HandleDeleteCollapsedSelectionAtCurrentBlockBoundary()`
to considering the content nodes to be joined part and doing join the nodes
part.
Differential Revision: https://phabricator.services.mozilla.com/D85567
`rightListElement` is not used, so, we can put it into smaller scope.
`leftListElement` is used for storing original `leftBlockElement` when
`newListElementTagNameOfRightListElement` is some. Therefore, we can
stop using it with caching the original `leftBlockElement` before
maybe modified its value.
Differential Revision: https://phabricator.services.mozilla.com/D85531
Now, the new method,
`WhiteSpaceVisibilityKeeper::MergeFirstLineOfRightBlockElementIntoLeftBlockElement()`
is the only user of the method so that we can get rid of it since it does enough
simple thing.
Differential Revision: https://phabricator.services.mozilla.com/D85530
And the name is wrong. It depends on `aPoint` whether the `<br>` element is
visible or invisible because it just scans preceding `<br>` element but
stop doing it when it meets a visible content. Therefore, this patch renames
it to explain what it does.
Differential Revision: https://phabricator.services.mozilla.com/D85528
CLOSED TREE
We don't need these macros anymore, for two reasons:
1. We have static analysis to provide the same sort of checks via `MOZ_RAII`
and friends.
2. clang now warns for the "temporary that should have been a declaration" case.
The extra requirements on class construction also show up during debug tests
as performance problems.
This change was automated by using the following sed script:
```
# Remove declarations in classes.
/MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER/d
/MOZ_GUARD_OBJECT_NOTIFIER_INIT/d
# Remove individual macros, carefully.
{
# We don't have to worry about substrings here because the closing
# parenthesis "anchors" the match.
s/MOZ_GUARD_OBJECT_NOTIFIER_PARAM)/)/g;
s/MOZ_GUARD_OBJECT_NOTIFIER_PARAM_TO_PARENT)/)/g;
s/MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)/)/g;
s/MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL)/)/g;
# Remove the longer identifier first.
s/MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_TO_PARENT//g;
s/MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM//g;
}
# Remove the actual include.
\@# *include "mozilla/GuardObjects.h"@d
```
and running:
```
find . -name \*.cpp -o -name \*.h | grep -v 'GuardObjects.h' |xargs sed -i -f script 2>/dev/null
mach clang-format
```
Differential Revision: https://phabricator.services.mozilla.com/D85168
We don't need these macros anymore, for two reasons:
1. We have static analysis to provide the same sort of checks via `MOZ_RAII`
and friends.
2. clang now warns for the "temporary that should have been a declaration" case.
The extra requirements on class construction also show up during debug tests
as performance problems.
This change was automated by using the following sed script:
```
# Remove declarations in classes.
/MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER/d
/MOZ_GUARD_OBJECT_NOTIFIER_INIT/d
# Remove individual macros, carefully.
{
# We don't have to worry about substrings here because the closing
# parenthesis "anchors" the match.
s/MOZ_GUARD_OBJECT_NOTIFIER_PARAM)/)/g;
s/MOZ_GUARD_OBJECT_NOTIFIER_PARAM_TO_PARENT)/)/g;
s/MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)/)/g;
s/MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL)/)/g;
# Remove the longer identifier first.
s/MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_TO_PARENT//g;
s/MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM//g;
}
# Remove the actual include.
\@# *include "mozilla/GuardObjects.h"@d
```
and running:
```
find . -name \*.cpp -o -name \*.h | grep -v 'GuardObjects.h' |xargs sed -i -f script 2>/dev/null
mach clang-format
```
Differential Revision: https://phabricator.services.mozilla.com/D85168
The previous patch changed target of white-space normalization. That caused
`editor/libeditor/tests/test_bug1425997.html` starting to failed with new
exception. What happens in the test is, selection is collapsed into comment
node (I think that this behavior is also a bug though) after second
`document.execCommand("delete")`, and then, failed to delete anything
and throws the exception. For fixing this issue, the most simple approach is
make `HTMLEditor::HandleDeleteAroundCollapsedSelection()` stop handling
deletion when caret position is not editable. So, unfortunately, this
patch changes traditional path too.
Depends on D84948
Differential Revision: https://phabricator.services.mozilla.com/D84949
Blink normalizes preceding white-spaces of deleting range only when the range is
NOT remove last character or there are no following text nodes. If removing
last character at first text node which is followed by another text node, Blink
normalize white-spaces at start of the following text node.
This behavior seems odd, but we should follow the Blink's behavior for better
compatibility.
Note that new test failures in
`white-spaces-after-execCommand-forwarddelete.tentative.html` is caused by
that forward deletion is handled by existing path instead of the new handler.
Therefore, it's no problem for now.
Additionally, this change causes a mochitest
(`editor/libeditor/tests/test_bug1425997.html`) and a crash test
(`editor/libeditor/crashtests/1424450.html`) becoming oranges since their
DOM tree becomes different after applying this patch. The oranges will
be fixed by the following patches.
Depends on D84947
Differential Revision: https://phabricator.services.mozilla.com/D84948
The difference between `HTMLEditor::HandleDeleteCollapsedSelectionAtTextNode()`
and `HTMLEditor::HandleDeleteCollapsedSelectionAtWhiteSpace()` is whether
it treats surrogate pair or not. And it's easy to handle it in `WSRunScanner`.
Therefore, they can handle all deletion in a text node.
Some new failures are caused by that we normalize preceding white-spaces of
deletion range but Blink does not to it. These failures will be fixed by
part 5.
Depends on D84946
Differential Revision: https://phabricator.services.mozilla.com/D84947
For the following patches, it's nicer that the API has optional behavior which
removes new empty text node or empty parent elements because directly removing
parent element can reduce the number of transactions and avoid storing text
with `DeleteTextTransaction()`.
Depends on D84944
Differential Revision: https://phabricator.services.mozilla.com/D84945
`InputEvent.getTargetRange()` should have actual delete range. It means that
the range should contain the invisible leading/trailing white-spaces which
need to be removed for keep them invisible, but should not contain the range
of normalizing white-space sequence because they are not target of edit action
indicated by `InputEvent.inputType`.
So, we can use new path which uses the new white-space normalizer for
computing the value of `InputEvent.getTargetRanges()` because difference of
white-space normalizer shouldn't affect the deleting ranges (although, some
existing path calls `DeleteNodeIfInvisibleAndEditableTextNode()` later so that
the new method, `ComputeRangeInTextNodesContainingInvisibleWhiteSpaces()`, does
not exactly same thing, but the result shouldn't become different in usual
cases). This new path can test with some WPTs under `editing/other`.
This patch creates new backspace/delete key handler when caret is at next
to a white-space as `HTMLEditor::HandleDeleteTextAroundCollapsedSelection()`
and creates helper methods of `WSRunScanner` to treat invisible leading and
trailing white-spaces.
Note that new failures are caused by the difference whether adjacent white-space
sequence at deletion is normalized or not in edge cases. They will be fixed
by the part.5.
Depends on D84943
Differential Revision: https://phabricator.services.mozilla.com/D84944
With the previous patch, 2 bugs become visible.
One is a `MOZ_ASSERT` in
`WhiteSpaceVisibilityKeeper::MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange()`
is wrong. When `TextFragmentData::GetReplaceRangeDataAtEndOfDeletionRange()`
returns a replace range, it may set the start to inclusive next character of
end to delete. So, start of replace range may start after end to delete when
container of end to delete is not a text node and its next editable char point
is an ASCII white-space. Therefore, this patch rewrite it with
`MOZ_ASSERT_IF` and `EditorDOMPoint::IsInTextNode()`.
The other is `TextFragmentData::GetReplaceRangeDataAtEndOfDeletionRange()`
and `TextFragmentData::GetReplaceRangeDataAtStartOfDeletionRange()` may call
`TextFragmentData::GetFirstASCIIWhiteSpacePointCollapsedTo()` and
`TextFragmentData::GetEndOfCollapsibleASCIIWhiteSpaces()` with a point in
preformatted text node. Therefore, this patch adds the check into them.
Depends on D84323
Differential Revision: https://phabricator.services.mozilla.com/D84943
Traditionally, `WSRunScanner` (and formerly `WSRunObject`) treats all text
nodes in its range when scan start container node has preformatted style.
This means that when starting start from start or end of preformatted text node
or inline element, it treats adjacent white-spaces which is not preformatted
as preformatted.
This patch fixes this issue. Because of the fix of preceding patches,
`BoundaryData` stops scanning if it meets preformatted text node. So,
`BoundaryData` can store whether the scanners found a preformatted character.
Therefore, if one of `BoundaryData`s is marked as "preformatted", it means
the range contains a preformatted character. And keep referring the style
of scan start point if there is no visible text nodes around it.
Note that the above change causes new failures with `<listing>` element which
should be treated as `<pre>` element, but `HTMLEditUtils` treats it as
non-container inline element.
https://html.spec.whatwg.org/multipage/dom.html#elements-in-the-dom:listing
Therefore, this patch changes the definition of `<listing>` element and
`<xmp>` element which is also mentioned in the above link to container
element etc. `<listing>` element is treated exactly same as `<pre>`,
therefore, the new definition is same as `<pre>`, but `<xmp>` does not
allow any tags after opens it (i.e., even if its close tag). Therefore,
`<xmp>` definition is different from `<listing>` and `<pre>` elements'
definition.
Differential Revision: https://phabricator.services.mozilla.com/D84323
The check does not make sense because `TextFragmentData`'s range may contain
multiple text nodes which may have different style.
With the previous patch, the range won't cross characters in preformatted
text nodes. Therefore, their result won't contain preformatted characters
right now. So, we can just drop the unnecessary check from them.
Differential Revision: https://phabricator.services.mozilla.com/D84320
Simply, if a white-space is in a preformatted text node, we shouldn't modify
it for normalizing. This patch adds such checks to various points in
`WSRunObject.cpp`.
Differential Revision: https://phabricator.services.mozilla.com/D84319
Currently, `TextFragmentData` stores whether the scan start point is
preformatted or not. However, it does not make sense because the class
may contain other text nodes which may have different style in its range.
The main job of `TextFragmentData` is managing white-spaces as visible
sequence or invisible sequence. So, preformatted white-space should be
treated as visible character because of not collapsible with adjacent
formatted ASCII white-spaces.
First of all, this patch its initializer stop scanning white-spaces if
it meets non-empty preformatted text node.
Note that the new failures are caused by the difference whether which
white-space sequence should be normalized when modifying text at text node
or inline element boundary. This difference should be fixed in another
bug because our new normalizer does not handle this same as Blink for now.
Differential Revision: https://phabricator.services.mozilla.com/D84317
Having two classes in the inheritance chain inherit from SupportsWeakPtr
now won't compile, but you can use WeakPtr<Derived> when any base class
inherits from SupportsWeakPtr.
Differential Revision: https://phabricator.services.mozilla.com/D83674
They return copy of cached `EditorDOMRange` instance. So, they can return
const-reference rather than copy of stored data.
Differential Revision: https://phabricator.services.mozilla.com/D83919
With this patch, the creation cost of `TextFragmentData` is minimized, but
`MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange()` can work
with the latest DOM tree information at touching start of the deleting range
even after touching end of the range.
Depends on D83228
Differential Revision: https://phabricator.services.mozilla.com/D83229
This is what the original method should've done. Mutation event listeners may
change the DOM tree at the first modification, but it hasn't checked whether
the range to delete or replace around start of deleting range is still valid
or not.
Depends on D83226
Differential Revision: https://phabricator.services.mozilla.com/D83228
For representing delete or replace data which have the range and replace string,
this patch creates `ReplaceRangeDataBase` class which is stack-only, has
`EditorDOMRange` or `EditorDOMRangeInTexts` as the range and has replace string
which can be empty string.
Then, `MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange()` should
modify the DOM tree by itself. With the following patch, the behavior is
corrected for multiple DOM tree changes.
Depends on D83224
Differential Revision: https://phabricator.services.mozilla.com/D83226
This patch is preparation for easier to review.
This patch duplicates the body of
`MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange()` and calls the
copies from new `MakeSureToKeepVisibleStateOfWhiteSpacesAroundDeletingRange()`.
The one is for handling start of deleting range, and the other is for handling
end of deleting range.
As you see, this patch and the following patches do not work because of the
duplicated code. Perhaps, part 2 works if there is no mutation event listeners.
Part 9 works even if there is mutation event listeners which touch the DOM
tree in the worst case (when it touches both end of delete range and start of
delete range, and around the start of deleting range is modified by JS).
Finally, part 9 takes back the original performance.
Depends on D82715
Differential Revision: https://phabricator.services.mozilla.com/D83217
Now, these classes are used only by `TextFragmentData` and they can be not
exposed. Therefore, we should hide them with making them private nested
classes of `TextFragmentData`.
Differential Revision: https://phabricator.services.mozilla.com/D82714
Although the new name is long, but I have no better idea. The class's purpose
is to keep white-space visibility around modifying DOM position. Therefore,
I use "keeper" for the name.
Differential Revision: https://phabricator.services.mozilla.com/D82713
Similar to the previous patch, `WSRunObject::NormalizeWhiteSpacesAround()` is
a wrapper to create `WSRunObject` instance for calling
`NormalizeWhiteSpacesAtEndOf()`, but it does not need to be `WSRunObject`'s
instance. Therefore, we can merge them.
Note that this renames the merged method to `NormalizeVisibleWhiteSpacesAt`.
Differential Revision: https://phabricator.services.mozilla.com/D82711
Now, it does not need to be a `WSRunObject` instance so that its wrapper to
create `WSRunObject` instance is not necessary.
Differential Revision: https://phabricator.services.mozilla.com/D82710
It's now can work with static helper methods and a `TextFragmentData` instance.
Therefore, this patch makes it a static method.
Note that it's always called with `nsIEditor::eNone` so that we can get rid of
the argument.
Differential Revision: https://phabricator.services.mozilla.com/D82705
Now, `mScanEndPoint` is not used. This patch removes it and clean up the
constructors of `WSRunScanner` and `WSRunObject`.
Differential Revision: https://phabricator.services.mozilla.com/D82703
It's simpler to make `WSRunScanner::InsertText()` take insertion point.
Then, it can do its jobs with `TextFragmentData` instance(s).
Differential Revision: https://phabricator.services.mozilla.com/D82702
`CreateVisibleWhiteSpacesData()` is now called multiple times and maybe called
after the DOM tree is modified even though it's a bug. Therefore, we should
make it store first result and return its reference instead.
Differential Revision: https://phabricator.services.mozilla.com/D82701
Same as the previous patch, it can be split to computation part and
modifying the DOM tree part. Then, the former can be in `TextFragmentData`
and the latter can be done by the caller which is only
`WSRunObject::InsertText()`.
Depends on D82699
Differential Revision: https://phabricator.services.mozilla.com/D82700
It's called by `WSRunObject::InsertText()` and `WSRunObject::InsertBreak()` and
it has 2 parts. One is, considering whether previous char is an NBSP or not
and whether it should be replaced with an ASCII white-space if it's followed
by visible character. The other is, doing the replacement. The latter code
is enough simple. Therefore, we can copy them into the callers. Then,
we can move the check logic into `TextFragmentData`.
Depends on D82698
Differential Revision: https://phabricator.services.mozilla.com/D82699
They work with a `TextFragmentData` instance, and the following patches
require to run it without `WSRunScanner`/`WSRunObject` instances.
Therefore, this patch moves them into `TextFragmentData`.
Depends on D82695
Differential Revision: https://phabricator.services.mozilla.com/D82697
This patch makes `WSRunScanner` have `TextFragmentData const mTextFragmentData`
instead of 2 `BoundaryData`s, a `NoBreakingSpaceData` and a `bool` storing
whether it's preformatted or not.
Depends on D82694
Differential Revision: https://phabricator.services.mozilla.com/D82695
For making `WSRunScanner::BoundaryData` independent from `WSRunScanner`,
its initializer should be in the class itself as static factory methods.
Depends on D82295
Differential Revision: https://phabricator.services.mozilla.com/D82693
Now, we can call `WSFragment` `VisibleWhiteSpacesData`. Then, the methods of
`WSRunObject` which take `WSFragment` as their arguments become clearer what
they mean.
Differential Revision: https://phabricator.services.mozilla.com/D82295
Now, `WSFragment` instances are created only by
`TextFragmentData::CreateWSFragmentForVisibleWhiteSpaces()`. Therefore, it's
always visible.
Additionally, this patch hides `WSFragment` constructors from the others too.
Differential Revision: https://phabricator.services.mozilla.com/D82294
Now, we know that it returns `WSFragment` for visible white-spaces and may
lie position in the line. Therefore, we should rename it as just representing
visible white-spaces.
Differential Revision: https://phabricator.services.mozilla.com/D82292
Now, nobody uses `WSRunScanner::FindNearestRun()` so that we can remove it.
Then, there is no users of `WSRunScanner:mFragments`. Therefore, we can remove
the member, accessors and initializers.
Differential Revision: https://phabricator.services.mozilla.com/D82291
Unfortunately, remaining code which use `beforeRun` and `afterRun` of
`WSRunObject::PreareToDeleteRangePriv()` is completely broken. It tries to
do what the new utility methods say, but as you see in the method, the
checking code does not make sense. For now, we should keep this broken
check even with the expensive DOM point comparisons. I hope that this does
not harm any benchmark score.
Note that I tested this with all automated tests with comparing the result
of these methods with `MOZ_ASSERT()` like this:
https://hg.mozilla.org/try/rev/29cb7840e404473a41d2d1fbdd229f762ccac5d3
So, I think that this is enough safe because the most edge cases are tested
by the first patch in this bug and `editing/run/(forwarddelete|delete).html`
of WPT.
Differential Revision: https://phabricator.services.mozilla.com/D82290
Similar to the previous changes, `WSRunObject::PrepareToDeleteRangePriv()`
can use `TextFragmentData::CreateWSFragmentForVisibleAndMiddleOfLine()` and
result of comparing with deleting range.
Differential Revision: https://phabricator.services.mozilla.com/D82288
For handling composition, `WSRunObject::InsertText()` rescan white-spaces at
end of replacing range. However, in most cases, `mScanStartPoint` and
`mScanEndPoint` are same. Therefore, we can save the runtime cost of the
rescan easier than the old design.
Differential Revision: https://phabricator.services.mozilla.com/D82287
I realized that `WSFragment::MarkAsVisible()` is called only by
`TextFragmentData::CreateWSFragmentForVisibleAndMiddleOfLine()`. Therefore,
`beforeRun && !beforeRun->IsStartOfHardLine() && beforeRun->IsVisible()` can be
handled with `pointPositionWithVisibleWhiteSpacesAtStart` and
`afterRun && !afterRun->IsEndOfHardLine() && afterRun->IsVisible()` can be
handled with `pointPositionWithVisibleWhiteSpacesAtEnd`. So, we can make
`WSRunObject::InsertText()` stop using `FindNearestFragment()` to compute
inserting string.
Differential Revision: https://phabricator.services.mozilla.com/D82286
Similar to `WSRunObject::InsertBreak()`, `WSRunObject::InsertText()` can use
the new APIs for `InsertBreak()` even though the name is odd for `InsertText()`,
but the mismatching is caused by odd logic of it. If replacing range starts
from and/or ends by middle of invisible white-space sequence, all of the
invisible white-spaces should be removed to prevent they become visible.
However, we shouldn't change any behavior in this bug.
Differential Revision: https://phabricator.services.mozilla.com/D82285
It removes some invisible leading and/or trailing white-spaces when it inserts
`<br>` element into the invisible white-space sequence. It currently checks
whether the insertion point is in invisible leading and trailing white-spaces
or not with `FindNearestFragment()`, but we can do same thing with
comparing the insertion point with the result of
`TextFragmentData::GetInvisibleLeadingWhiteSpaceRange()` and
`TextFragmentData::GetInvisibleLeadingWhiteSpaceRange()`. However, current
implementation does not make sense because:
- It checks trailing white-spaces with `!IsEndOfHardLine()` and
`IsStartOfHardLine()`, but this means that it does ignores invisible
white-spaces which are the only content in a line.
- It checks leading white-spaces with `!IsStartOfHardLine()` and
`IsEndOfHardLine()`, so, this also ignores invisible white-spaces which
are the only content in a line.
- The important thing of the logic is prevent that invisible leading and
trailing white-spaces become visible with new `<br>` element, but this
is done only for trailing white-spaces.
Differential Revision: https://phabricator.services.mozilla.com/D82283
It looks for same `WSFragment` twice with comparing text and what it looks for
is the instance whose `IsVisibleAndMiddleOfLine()` returns `true`.
Additionally, doing twice is for checking only whether the split point is
start or end of the range. Therefore, we can make it simpler with using
`TextFragmentData::CreateWSFragmentForVisibleAndMiddleOfLine()`.
Differential Revision: https://phabricator.services.mozilla.com/D82281
They return a point in an `WSFragment` whose `IsVisibleAndMiddleOfLine()`
returns `true` or start/end of the range. Therefore, they can ignore the
other `WSFragment`s and save comparing points to at most twice.
Differential Revision: https://phabricator.services.mozilla.com/D82280
It scans all `WSFragment`s in the array and call `NormalizeWhiteSpacesAtEndOf()`
only with visible instance. However, as we know, there is at most one such
instance. Therefore, it does not need to scan the fragments (i.e., don't need
to create all fragments).
Differential Revision: https://phabricator.services.mozilla.com/D82279
Most methods handling something with `WSFragment` work only with instances
whose `IsVisibleAndMiddleOfHardLine()` return true, and currently, such
instance exists only one at most in the array. Therefore, they just want
an API to get such fragment.
Differential Revision: https://phabricator.services.mozilla.com/D82276
`Scrub()` does remove leading white-spaces and trailing white-spaces if
there is. So, it does not require `WSFragment` anymore.
Differential Revision: https://phabricator.services.mozilla.com/D82275
This is minimally invasive. In the following parts, methods will be
moved to it and potentially other methods will be extracted.
Differential Revision: https://phabricator.services.mozilla.com/D82384
Different from `GetInvisibleTrailingWhiteSpaceRange()`, it always returns
the range even if it's collapsed (i.e., there is no leading white-space).
Differential Revision: https://phabricator.services.mozilla.com/D82274
One of the `WSFragment`users' purpose is, they remove invisible white-spaces
when there are. So, `TextFragmentData` should have API to retrieve the
ranges and use them for initializing `WSFragment` which represents leading
or trailing white-spaces. For making this patch smaller as far as possible,
these APIs implements only the case when there is no NBSP.
For result of the new API, this creates a template class, `EditorDOMRangeBase`,
whose boundary type is `EditorDOMPointBase`. Its methods are named from
`nsRange`'s same methods.
Differential Revision: https://phabricator.services.mozilla.com/D82272
This patch just creates new stack only class and make it initializes
`WSRunScanner::mFragments` instead of `WSRunScanner`.
Differential Revision: https://phabricator.services.mozilla.com/D82270
For making easier to understand the contents of `WSRunScanner::mFragments`,
we should stop using `InitializeWithSingleFragment()` for now because it's
designed for doing same things a lot, but used only by 2 users and they set
different parameters so that it does not do same things for the callers.
Depends on D82268
Differential Revision: https://phabricator.services.mozilla.com/D82269
`nsINode::InsertBefore()` removes inserting node from a document if it's in
a document including different document. In this case, `UpdateReflectorGlobal`
in BindingUtils.cpp calls `ErrorResult::MightThrowJSException()`, but editor
never throws exception with `ErrorResult`. Therefore, editor needs to call
`ErrorResult::WouldReportJSException()` explicitly if the inserting node may
be in another document.
As far as I checked, it can happen only when undoing or redoing a transaction.
Therefore, this patch touches only transaction classes.
Depends on D81683
Differential Revision: https://phabricator.services.mozilla.com/D81684
Although their callers may want to remove empty text nodes around white-space
sequence, but for now, we should make them not return empty text node because
the former's name means so, and the latter should behave similarly for
consistency.
Differential Revision: https://phabricator.services.mozilla.com/D81683
In most methods of `WSRunScanner`, `WSFragment`s are never used. Therefore,
this patch makes them created when they are necessary.
Depends on D80315
Differential Revision: https://phabricator.services.mozilla.com/D80638
`WSFragment` is created at least one instance, and at most 3 instances per
`WSRunScanner` instance. They can be managed with `AutoTArray` simpler and
we can avoid heap allocation with this approach.
Differential Revision: https://phabricator.services.mozilla.com/D80314
Part 1-* rewrite `WSRunScanner::GetRuns()` with early-return style, but it's
hard to review if changing it with a patch.
This just swaps the `if` and `else` block because the `else` block is
shorter than the `if` block.
Depends on D79973
Differential Revision: https://phabricator.services.mozilla.com/D80311
With the previous patches, we know that the loops in
`WSRunScanner::InitializeRangeStart()` and `WSRunScanner::InitializeRangeEnd()`
rarely run twice and more. Therefore, we can make use recursive calls instead
of the loop.
Depends on D79970
Differential Revision: https://phabricator.services.mozilla.com/D79971
Similar to the previous patch, this patch moves the last `else` block of
the loops in `WSRunScanner::InitializeRangeStart()` and
`WSRunScanner::InitializeRangeEnd()` to before the text node handling case.
This makes that clearer that the loops are continued only when text node
has no text or only white-spaces.
Depends on D79969
Differential Revision: https://phabricator.services.mozilla.com/D79970
Their topmost `else` blocks mean there is no visible content before/after the
point. In this case, the initialization is simpler. So, they should be
handled first, and make the other blocks outdented.
Depends on D79966
Differential Revision: https://phabricator.services.mozilla.com/D79969
There are duplicated code in both `WSRunScanner::InitializeRangeStart()`
and `WSRunScanner::InitializeRangeEnd()`. They scan text node to store
first and last NBSP positions and initialize start/end with found visible
character position. This patch makes the loop clearer.
Depends on D79965
Differential Revision: https://phabricator.services.mozilla.com/D79966
Despite the name, it stores range of white-space sequence and/or start/end
reason. Initializing start and end are completely independent. Therefore,
we can move them into independent methods.
Depends on D79964
Differential Revision: https://phabricator.services.mozilla.com/D79965
I guess that the `Maybe` is `mOffset` of `EditorDOMPointBase` because new
`Maybe::value()`s are called with checking `Maybe::isSome()`. So, accessing
`EditorDOMPointBase::mOffset` newly should cause the assertion.
Then, I found a new caller `IsCharASCIISpace()` which calls `Char()` without
validation here:
https://hg.mozilla.org/mozilla-central/diff/289c293af80b12744b5d35c5b8427ba8d8ebf13e/editor/libeditor/WSRunObject.cpp#l1.383
That could be unset, but I cannot reproduce it, but I succeeded to reproduce
similar assertion hit with using empty text node (see the crashtest). I hope
this fixes the original crash too.
Differential Revision: https://phabricator.services.mozilla.com/D79816
This patch tries to implement Blink-compat white-space normalizer for
`HTMLEditor`.
It's difficult to list up our traditional white-space normalization rules
because `WSRunObject` touches white space sequence only when there is not
acceptable case, e.g., an ASCII white-spaces will be adjacent to another
one, and replaces only unacceptable white-space only. Therefore, whether
white-space sequence may start with either an ASCII white-space or an NBSP.
On the other hand, Blink and WebKit makes white-space sequence always
starts with an NBSP or an ASCII white-space (unfortunately, they behave
differently!). So, for web-compat, we should simulate Blink's behavior
because either behavior is reasonable but Blink have more market share.
This patch simply adds new white-space normalization path for the new one,
and it's switchable with a pref, and still disabled by default.
The other reason why we should do this is, our traditional white-space
normalizer touches the DOM a lot of times per edit action, and the timing
is both before and after touches the DOM tree. Therefore, it's difficult
to compute actual deleting range for `InputEvent.getTargetRanges()` and
touching a lot of times causes running mutation event listeners a lot and
creates a lot of transaction class instances. So, new one have a lot of
merits:
1. Improve web-compat
2. Improve the peformance
3. Improve the security
4. Improve the footprint (but this is now worse then traditional one)
5. Simplify the implementation
The new normalizer is mostly implemented with only 3 `HTMLEditor` methods.
One is `HTMLEditor::DeleteTextAndNormalizeSurroundingWhiteSpaces()`. This is
semi-public method for the edit action handlers. This takes a range with
2 `EditorDOMPoinInText` to delete the range simply. This also replaces
surrounding white-space sequence if necessary. For inserting text case,
this method also handles only white-space normalization when it's called
with collapsed range, i.e., same `EditorDOMPointInText`. This tries to use
`RepaceTextWithTransaction()` as far as possible to reduce creation cost of
transaction classes and the footprint.
Another one is `HTMLEditor::ExtendRangeToDeleteWithNormalizingWhiteSpaces()`.
This tries to extend the given range to normalize surrounding white-spaces.
This is currently not optimized for footprint because this may include
white-spaces which do not need to be replaced. This optimization should be
done before shipping, but for now, enabling `InputEvent.getTargetRanges()` in
Nightly channel is more important. So that it should be done in a follow-up
bug.
The other is `HTMLEditor::GenerateWhitepaceSequence()`. This creates
normalized white-space sequence with surrounding character information.
For keeping this method simple as far as possible, we shouldn't optimize
the range of generation even in follow-ups.
Finally, the white-space sequence is not tested in mochitests, so that we
can enable this new normalizer when we run mochitests under
`editor/libeditor/tests`. However, WPT has some tests. We should keep
them running with current normalizer for checking regression. Instead,
we should enable the pref only for the new WPT added by the previous patch.
Depends on D78655
Differential Revision: https://phabricator.services.mozilla.com/D78656
I realized that there is no word "whitespace" in formal English. This patch
replaces it with "white-space" in comments, and change method names to use
"WhiteSpace".
Depends on D78654
Differential Revision: https://phabricator.services.mozilla.com/D78655
Callers of `WSRunObject::GetASCIIWhitespacesBounds()` may want to scan only
previous or next whitespaces. Therefore, we can split it to save creation
cost of `EditorDOMPointInText`.
Additionally, this makes them scan whitespace sequence in a text node until
hitting its end for avoiding to use expensive API of `WSRunScanner`.
Differential Revision: https://phabricator.services.mozilla.com/D77986
This change was only needed when the previous version of the patch for Bug 1632425 wanted to move things into contextmenu event handler. This patch reverts the change as 1) the suggested behavior never landed and 2) opening context menu in a test can cause conflict with other tests.
Differential Revision: https://phabricator.services.mozilla.com/D78673
Fix of bug 1320229 allowed to paste longer text than `maxlength` attribute of
`<input>` and `<textarea>` because it was thought that the longer text causes
"too long" invalidate state, makes users notified and prevent to submit
form data.
However, according to Bug 1636855 comment 7 (*1), it breaks a major enterprise
web app, SAP, at least because it sends form data without checking validity of
each form data and discards invalid data on server side silently.
According to bug 1636855 comment 24 (*2), one of new behavior's fault is
on Gecko side too. The style of `<input>` element or `<textarea>` element
which has too long text after pasting is changed when it loses focus.
Therefore, users can post the data before they know pasted data is too
long if sending the form data with `Enter` key or something immediately
after pasting (i.e., without moving focus) web apps handle it by themselves.
On the other hand, the original bug report, bug 1320229, should be solved in
the future especially in password field because users may register password
which is cut by `maxlength` silently and they don't use builtin password
manager, only the pasted password is saved, and then, they won't be able to
login as the account. This is really long standing issue of the web forms.
An article (*3) warned this to web developers in 2011. Therefore, we should
keep going advance for solving this issue at least in Nightly channel to get
more feedback from testers and web developers.
1 https://bugzilla.mozilla.org/show_bug.cgi?id=1636855#c7
2 https://bugzilla.mozilla.org/show_bug.cgi?id=1636855#c24
3 https://www.christophermanning.org/writing/dont-use-maxlength-on-password-inputs
Differential Revision: https://phabricator.services.mozilla.com/D78613
According to the telemetry data, almost all web apps in the word do not use
mutation event listeners, but if they are used, editor needs to check
whether modifying node is modified by web apps. The checking cost may not
cheap in some cases. Therefore, we check whether the window has at least one
mutation event listeners or not before non-cheap checking.
However, like actual web apps, most our mochitests and WPT do not add
mutation event listeners. Therefore, such additional checking code is not
tested even though they should work as exactly same as the case when
there are mutation event listeners but they don't touch the DOM.
This patch makes it always return only in debug build for checking the
complicated path.
Differential Revision: https://phabricator.services.mozilla.com/D78173
There's no use case for stateful comparators, so they can be just plain
function pointers.
This is used in some hot places like CSS selector matching.
Differential Revision: https://phabricator.services.mozilla.com/D77084
We were dealing with it correctly when switching display from e.g. block
to inline, or such. But we were not dealing with it when the node was
undisplayed.
Handle it properly, and free one frame bit while at it. We can't really
do this for ManualNAC (the editor resizers) because they request a
reframe explicitly.
Differential Revision: https://phabricator.services.mozilla.com/D76679
In favor of the NativeAnonymous versions which they forward to.
Done automatically with:
rg -l 'IsInAnonymousSubtree' | xargs sed -i 's/IsInAnonymousSubtree/IsInNativeAnonymousSubtree/g'
And removing the function definitions afterwards.
Differential Revision: https://phabricator.services.mozilla.com/D76681
Masayuki Nakano
Sonia Singla
Mihai Alexandru Michis
Chris Fronk
Mirko Brodesser
Edgar Chen
Dorel Luca
Kagami Sascha Rosylight
Kagami Sascha Rosylight
Cosmin Sabou
Butkovits Atila
Sylvestre Ledru
Andreas Farre
Nathan Froyd
tkhan
Emilio Cobos Álvarez
Kashav Madan
Simon Giesecke
Razvan Maries
Bogdan Tara
Kris Maglione
Gijs Kruitbosch
Ping Chen