Although we use CSS property for Inline table editing UI, we use edit
transaction for it unfortunately. When hiding this UI, since removing
element doesn't use edit transaction, transaction will be canceled before
showing this UI.
It is unnecessary to use edit transaction for UI, so we shouldn't use it.
Differential Revision: https://phabricator.services.mozilla.com/D90056
This is causes taking back bugs in
`WSRunScanner::GetNewInvisibleLeadingWhiteSpaceRangeIfSplittingAt()` and
`WSRunScanner::GetNewInvisibleTrailingWhiteSpaceRangeIfSplittingAt()` (I.e.,
they were fixed by bug 1647556). Therefore, this removes the odd `if` blocks
which are pointed with `XXX` comment for avoiding "regressions" (without them,
some WPTs for `InputEvents.getTargetRanges()` become failure).
Depends on D89582
Differential Revision: https://phabricator.services.mozilla.com/D89869
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
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
`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
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
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
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
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
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
`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 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
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
Currently, when `HTMLEditor` replaces text in a text node, `HTMLEditor`
creates a set of `DeleteTextTransaction` and `InsertTextTransaction`.
However, this has bad impact for footprint and causes the callers messy.
This patch creates `ReplaceTextTransaction` instead and
`HTMLEditor::ReplaceTextWithTransaction()` as its wrapper. Unfortunately,
this becomes not calling `nsIEditActionListener::DidDeleteText()`, however,
this is not used by mozilla-central, comm-central nor BlueGriffon. IIRC,
it was not removed for some legacy addons of Thunderbird. Therefore, it
must be okay to remove it.
Differential Revision: https://phabricator.services.mozilla.com/D76078
Makoto Kato
Masayuki Nakano
Timothy Nikkel
Kagami Sascha Rosylight
Kagami Sascha Rosylight
Aaron Klotz
Mirko Brodesser
Edgar Chen
Dorel Luca
Cosmin Sabou
Sonia Singla
tkhan
Kashav Madan
Bogdan Tara
Kris Maglione