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
