It'll fail to join them when its utility methods actually join them.
However, it should be considered earlier timing because the preparation
DOM tree changes before joining them causes creating odd tree (see the
removed cases of the test). Therefore, we should make them stop handling
deletion before modifying the DOM tree.
Depends on D89579
Differential Revision: https://phabricator.services.mozilla.com/D89580
This makes `AutoInclusiveAncestorBlockElementsJoiner` stores whether `Run()`
will return "ignored" or not when `Prepare()` is called. And this patch adds
`NS_ASSERTION()` to compare the result of `Run()` and the guess.
Note that this shouldn't used for considering whether its user call or not
`Run()` actually for the risk of regressions and if we do it with current
implementation, some web apps may be broken if they do modify the DOM tree
at white-space adjustment before joining the left and right block elements.
The method will be used by `ComputeTargetRanges()` which will be implemented
by bug 1658702.
Depends on D89578
Differential Revision: https://phabricator.services.mozilla.com/D89579
When its `Run()` join the block elements, even if it won't move any content
nodes in first line, it may notify the callers as "handled" when there is a
preceding invisible `<br>` element since it needs to delete it in any cases.
Therefore, whether there is or not a preceding invisible `<br>` element is
important to compute target ranges since if there is one, `Run()` won't return
"ignored" and the callers won't delete leaf content instead.
Note that the new assertions are not hit in any tests, but due to searching
preceding invisible `<br>` element in `Prepare()`, expected assertion
count of 2 tests is increased.
Depends on D89577
Differential Revision: https://phabricator.services.mozilla.com/D89578
The relation is important when the class handles both joining them and computing
target ranges. Additionally, it's required to consider whether it can join the
block elements. So, it should store the relation when `Prepare()` is called.
Depends on D89576
Differential Revision: https://phabricator.services.mozilla.com/D89577
Unfortunately, `HTMLEditor::MoveOneHardLineContents()`,
`HTMLEditor::MoveChildrenWithTransaction()` and
`HTMLEditor::MoveNodeOrChildrenWithTransaction()` return strict result whether
at least one node is moved or not. Therefore, we need to scan the DOM tree
whether there is at least one content node which can be moved by them for
computing target ranges.
We cannot do same thing for ``HTMLEditor::MoveOneHardLineContents()` because
it split parent elements first and use `ContentSubtreeIterator` which lists
up topmost nodes which are completely in the range, but we need to compute
target ranges without splitting nodes at the range boundaries. Therefore,
this patch checks whether the line containing specified point has content
except invisible `<br>` element.
The others are simple. We can use same logic with them.
Finally, this adds `NS_ASSERTION()`s to check whether the computation is done
correctly at running any automated tests on debug build, and I don't see any
failure with them.
Depends on D89575
Differential Revision: https://phabricator.services.mozilla.com/D89576
When they return error, neither "handled" nor "canceled" state is referred.
So, for making the code simpler, we can make them return
`EditActionResult(NS_ERROR_*)` instead of `EditActionIgnored(NS_ERROR_*)`.
Depends on D89574
Differential Revision: https://phabricator.services.mozilla.com/D89575
The 2 of them always mark the edit action as "handled", but it's not important
when it's "canceled" or an error. Therefore, we can make the users simpler
as this patch does.
Depends on D89573
Differential Revision: https://phabricator.services.mozilla.com/D89574
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
