Now, `AutoInclusiveAncestorBlockElementsJoiner::Run()` is wrapped by a
small block in every caller. Therefore, `AutoTrackDOMPoint` for it can
be moved into the small blocks.
Depends on D89571
Differential Revision: https://phabricator.services.mozilla.com/D89572
For making the refactoring patch simpler, `Prepare()` considers the
`EditActionResult` value of its callers instead. However, this is odd
so that it just return whether the caller should keep working with
it or not as `bool` result. Then, for the additional information, whether
the caller should consume the edit action handling or not, this patch
adds a new method, `CanJoinBlocks()`.
Depends on D89440
Differential Revision: https://phabricator.services.mozilla.com/D89571
Doing rAF rAF flushApzRepaints is not enough to make sure that any potential scroll events are sent to content. The reason is that if the apz repaint request causes us to do scrolling on the main thread then the scrolling will be finished after flushApzRepaints, and the scroll event will be pending, but it's not sent until the next refresh driver tick. So we need to do at least one rAF after flushApzRepaints.
Differential Revision: https://phabricator.services.mozilla.com/D90035
I think the scrolls that zoom to focus input causes are giving us scroll events that we don't expect. I don't think there is a better way around this.
Differential Revision: https://phabricator.services.mozilla.com/D89993
The test fixes all fell into the follow categories:
A) The test uses requestAnimationFrame to wait one frame and expects scrolling to be complete. With the desktop zooming scrollbars in order for the scrolling to show up on the main thread we need to send the scroll request to the compositor and then hear back from it via an apz repaint request (apz callback helper). Waiting on requestAnimationFrame will complete the first part, but not necessarily the second part. The fix is to wait for a scroll event.
B) Switching tests to wait for scroll events exposes another problem: the test can do things that cause a scroll in order to setup the test (and that may not be obvious that it causes a scroll) before actually proceeding to do the test and do something that causes a scroll and then checks for the scroll change of the second thing. Waiting for a requestAnimationFrame would include both those scrolls without desktop zooming scrollbars, but if we wait for a scroll event we will get the scroll event for the first thing which we are not interested in. So we need to make sure scroll events are cleared out before waiting for any scroll events. We do this by waiting two requestAnimationFrame's and waiting for apz to be flushed. We also use this when a test does something and it wants to test that scrolling is not performed.
The main thing that causes scrolling that may not be obvious: calling node.focus(). With stacks like:
from test_scroll_per_page.html
```
#01: mozilla::ScrollFrameHelper::CompleteAsyncScroll(nsRect const&, mozilla::ScrollOrigin) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x47d6cc0]
#02: mozilla::ScrollFrameHelper::ScrollToWithOrigin(nsPoint, mozilla::ScrollMode, mozilla::ScrollOrigin, nsRect const*, nsIScrollbarMediator::ScrollSnapMode) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x47d7732]
#03: mozilla::layout::ScrollAnchorContainer::ApplyAdjustments() [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x4742913]
#04: mozilla::PresShell::FlushPendingScrollAnchorAdjustments() [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x4650069]
#05: mozilla::PresShell::ProcessReflowCommands(bool) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x465742b]
#06: mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x4656af8]
#07: mozilla::dom::Document::FlushPendingNotifications(mozilla::ChangesToFlush) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1a87d3c]
#08: mozilla::PresShell::ScrollContentIntoView(nsIContent*, mozilla::ScrollAxis, mozilla::ScrollAxis, mozilla::ScrollFlags) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x4652b96]
#09: nsFocusManager::ScrollIntoView(mozilla::PresShell*, nsIContent*, unsigned int) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1bedd1c]
#10: nsFocusManager::Focus(nsPIDOMWindowOuter*, mozilla::dom::Element*, unsigned int, bool, bool, bool, bool, bool, nsIContent*) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1be6be0]
#11: nsFocusManager::SetFocusInner(mozilla::dom::Element*, int, bool, bool) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1be212f]
#12: nsFocusManager::SetFocus(mozilla::dom::Element*, unsigned int) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1be32ba]
#13: mozilla::dom::Element::Focus(mozilla::dom::FocusOptions const&, mozilla::dom::CallerType, mozilla::ErrorResult&) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1aaf283]
#14: mozilla::dom::HTMLElement_Binding::focus(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x2d65f3b]
```
from editor/libeditor/tests/test_bug549262.html
```
#01: mozilla::ScrollFrameHelper::CompleteAsyncScroll(nsRect const&, mozilla::ScrollOrigin) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x47d6cc0]
#02: mozilla::ScrollFrameHelper::ScrollToWithOrigin(nsPoint, mozilla::ScrollMode, mozilla::ScrollOrigin, nsRect const*, nsIScrollbarMediator::ScrollSnapMode) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x47d7732]
#03: mozilla::PresShell::ScrollFrameRectIntoView(nsIFrame*, nsRect const&, mozilla::ScrollAxis, mozilla::ScrollAxis, mozilla::ScrollFlags) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x46541bc]
#04: mozilla::PresShell::DoScrollContentIntoView() [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x4653776]
#05: mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x4656b11]
#06: mozilla::dom::Document::FlushPendingNotifications(mozilla::ChangesToFlush) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1a87d3c]
#07: mozilla::PresShell::ScrollContentIntoView(nsIContent*, mozilla::ScrollAxis, mozilla::ScrollAxis, mozilla::ScrollFlags) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x4652b96]
#08: nsFocusManager::ScrollIntoView(mozilla::PresShell*, nsIContent*, unsigned int) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1bedd1c]
#09: nsFocusManager::Focus(nsPIDOMWindowOuter*, mozilla::dom::Element*, unsigned int, bool, bool, bool, bool, bool, nsIContent*) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1be6be0]
#10: nsFocusManager::SetFocusInner(mozilla::dom::Element*, int, bool, bool) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1be212f]
#11: nsFocusManager::SetFocus(mozilla::dom::Element*, unsigned int) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1be32ba]
#12: mozilla::dom::Element::Focus(mozilla::dom::FocusOptions const&, mozilla::dom::CallerType, mozilla::ErrorResult&) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x1aaf283]
#13: mozilla::dom::HTMLElement_Binding::focus(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) [/Users/tim/ffopt2/src/obj-x86_64-apple-darwin19.6.0/toolkit/library/build/XUL + 0x2d65f3b]
```
C) Several tests use nsIDOMWindowUtils advanceTimeAndRefresh/restoreNormalRefresh and expect scrolling to be done after a call to advanceTimeAndRefresh. This is basically A), advanceTimeAndRefresh does a refresh driver tick but doesn't allow a repaint request to come back to the main thread.
Differential Revision: https://phabricator.services.mozilla.com/D89403
See the bug for the complications that made me write this slightly hacky
fix... Other solutions definitely welcome.
Add a test, adjusted so it would fail without the change.
Differential Revision: https://phabricator.services.mozilla.com/D89835
`RangeBoundaryBase` stores a previous sibling of child node at offset with
`mRef`. Therefore, even if the callers check whether its instance points a
child node, `mRef` may be `nullptr` when it points first child of its container.
So, `GetNextSiblingOfChildAtOffset()` needs to handle the case.
This patch adds the crash case test into
`test_dom_input_event_on_htmleditor.html` because of a basic behavior.
For now, this patch adds 2 chunks which are coded with using same style as
previous ones. However, the test should be redesigned later for making
non-dependency of each chunk guaranteed. (The new chunks are completely
independent from previously running tests.)
Differential Revision: https://phabricator.services.mozilla.com/D89440
CLOSED TREE
Backed out changeset d3d67293f115 (bug 1623413)
Backed out changeset 75ed1b8a5c67 (bug 1623413)
Backed out changeset 0eef32d6a454 (bug 1623413)
The assertion added in part 77 isn't violated, but it needs exhaustive
analysis to ensure it's indeed intended to not be violated.
Differential Revision: https://phabricator.services.mozilla.com/D88258
This change is corresponding to the part:
https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/editor/libeditor/HTMLEditSubActionHandler.cpp#3143-3155
When caret is not adjacent the deleting character in bidi text, we may do
nothing except putting caret to the character. So, `ComputeRangesToDelete()`
shouldn't update the caret position since the caret position will be check
by its `Run()` later if `beforeinput` event is not canceled. For avoiding
the code duplication, this patch reimplements
`EditorBase::SetCaretBidiLevelForDeletion()` as a stack only class and
split the check and updating part from correcting the data.
Note that by the default pref, the new tests failed since it won't be
canceled, and the method still don't compute for deleting a character.
Differential Revision: https://phabricator.services.mozilla.com/D88378
This patch implements computation of target ranges for this part:
https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/editor/libeditor/HTMLEditSubActionHandler.cpp#3099-3141
This patch adds some utility methods for computing the ranges. Currently,
it's not yet standardized, but the other browser engines look for leaf content
of another block when blocks are joined (or a block is deleted like this case).
Therefore, we follow the behavior basically, but different from the other
browsers, we should include invisible white-spaces into the range when they
are included. That avoids the invisible white-spaces become visible when
web apps do something instead of us. Note that utility methods have the code,
but this patch does not use it because in this case, we just delete a empty
block ancestor, not join it with previous/next block.
Differential Revision: https://phabricator.services.mozilla.com/D88377
This patch implements computation of target ranges for this part:
https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/editor/libeditor/HTMLEditSubActionHandler.cpp#3099-3141
This patch adds some utility methods for computing the ranges. Currently,
it's not yet standardized, but the other browser engines look for leaf content
of another block when blocks are joined (or a block is deleted like this case).
Therefore, we follow the behavior basically, but different from the other
browsers, we should include invisible white-spaces into the range when they
are included. That avoids the invisible white-spaces become visible when
web apps do something instead of us. Note that utility methods have the code,
but this patch does not use it because in this case, we just delete a empty
block ancestor, not join it with previous/next block.
Differential Revision: https://phabricator.services.mozilla.com/D88377
In strictly speaking, we should use same computed target ranges for any edit
actions which causes removing non-collapsed selection. However, for now,
this patch makes only `DeleteSelectionAsAction()` because it's not so important
differences for shipping `beforeinput` in Nightly channel.
Differential Revision: https://phabricator.services.mozilla.com/D88376
The editor modules does QI too many times when it sets or removes some style
with `execCommand` or XPCOM API. Therefore, there should be an API to
retrieve `nsStyledElement` pointer from `nsINode*`.
Differential Revision: https://phabricator.services.mozilla.com/D87990
Although it starts to return error if it causes destroying the editor, but
it should not occur because it modifies new and orphan node and it shouldn't
kick any mutation event listeners. Therefore, this patch makes the callers
handle error as-is rather than ignoring errors except
`NS_ERROR_EDITOR_DESTROYED`.
Differential Revision: https://phabricator.services.mozilla.com/D87989
It should take `nsStyledElement&` instead of `const Element&`. Then, it won't
fail and will just return the result of `nsStyleElement::Style()`. Therefore,
we can get rid of it.
Note that this patch makes all its callers keep using strong pointer because
I'm not sure whether the layout APIs which are called with them are safe or
not.
Differential Revision: https://phabricator.services.mozilla.com/D87982
When it returns `EditActionIgnored()`,
`AutoDeleteRangesHandler::HandleDeleteAroundCollapsedRanges()` creates another
`AutoDeleteRangesHandler` instance to delete leaf content node in another
block element. However, this dependency makes developers understand the
behavior harder. Therefore, we should make the method do it instead.
Differential Revision: https://phabricator.services.mozilla.com/D87439
For avoiding infinite recursive calls, `AutoDeleteRangesHandler` returns
`EditActionIgnored()` when it needs to fall it back to
`DeleteRangesWithTransaction()` after modifying the DOM tree or the ranges
to delete. However, this makes developers understand the code harder.
And now, with making constructor of `AutoDeleteRangesHandler` manage the
recursive call state, we can make `HandleDeleteSelection()` stop handling
the fallback path.
Differential Revision: https://phabricator.services.mozilla.com/D87438
Even though their method names in stack trace become too long, but we can
guarantee that they are used only at handling deletion.
Differential Revision: https://phabricator.services.mozilla.com/D87433
This patch just moves them into the new stack only class and make each of them
take `HTMLEditor&` as their first argument.
Differential Revision: https://phabricator.services.mozilla.com/D87432
If `AutoSetTemporaryAncestorLimiter` sets ancestor limiter of the `Selection`,
existing range which is already cached by `AutoRangeArray` may be changed
into the new limiter.
Therefore, this patch makes `AutoSetTemporaryAncestorLimiter` take
`AutoRangeArray` optionally and reset it only when it sets new limiter for
the performance.
Differential Revision: https://phabricator.services.mozilla.com/D87820
The root cause of this bug is a bug of async `ScrollSelectionIntoView` that
is what it won't be canceled even if web app changes scroll position with
any API. Fixing it is really risky and this bug affects an existing website.
Therefore, this patch makes the constructors of `AutoPlaceholderBatch` take
whether do or do not `ScrollSelectionIntoView` when it's destructed. And
makes `HTMLEditor::SetInlinePropertyAsAction()` not request
`ScrollSelectionIntoView` for taking back the traditional behavior.
Note that this patch does not make it an optional argument because it's hard to
guess that it'll run `ScrollSelectionIntoView` automatically.
Differential Revision: https://phabricator.services.mozilla.com/D87819
Even though it hasn't normalize white-spaces before invisible `<br>` element,
it needs to do it for making them visible. Therefore, we should make it
use the new method in this case too.
Depends on D87030
Differential Revision: https://phabricator.services.mozilla.com/D87031
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
Masayuki Nakano
longsonr
Timothy Nikkel
Emilio Cobos Álvarez
Kagami Sascha Rosylight
Kagami Sascha Rosylight
Jonathan Kew
Mihai Alexandru Michis
Aaron Klotz
Mirko Brodesser
Chris Fronk
Simon Giesecke
Sonia Singla
Edgar Chen
Dorel Luca
Cosmin Sabou
Butkovits Atila
Sylvestre Ledru
Andreas Farre
Nathan Froyd
tkhan
Kashav Madan
Razvan Maries
Bogdan Tara
Kris Maglione