Part 1-* rewrite `WSRunScanner::GetRuns()` with early-return style, but it's
hard to review if changing it with a patch.
This just swaps the `if` and `else` block because the `else` block is
shorter than the `if` block.
Depends on D79973
Differential Revision: https://phabricator.services.mozilla.com/D80311
With the previous patches, we know that the loops in
`WSRunScanner::InitializeRangeStart()` and `WSRunScanner::InitializeRangeEnd()`
rarely run twice and more. Therefore, we can make use recursive calls instead
of the loop.
Depends on D79970
Differential Revision: https://phabricator.services.mozilla.com/D79971
Similar to the previous patch, this patch moves the last `else` block of
the loops in `WSRunScanner::InitializeRangeStart()` and
`WSRunScanner::InitializeRangeEnd()` to before the text node handling case.
This makes that clearer that the loops are continued only when text node
has no text or only white-spaces.
Depends on D79969
Differential Revision: https://phabricator.services.mozilla.com/D79970
Their topmost `else` blocks mean there is no visible content before/after the
point. In this case, the initialization is simpler. So, they should be
handled first, and make the other blocks outdented.
Depends on D79966
Differential Revision: https://phabricator.services.mozilla.com/D79969
There are duplicated code in both `WSRunScanner::InitializeRangeStart()`
and `WSRunScanner::InitializeRangeEnd()`. They scan text node to store
first and last NBSP positions and initialize start/end with found visible
character position. This patch makes the loop clearer.
Depends on D79965
Differential Revision: https://phabricator.services.mozilla.com/D79966
Despite the name, it stores range of white-space sequence and/or start/end
reason. Initializing start and end are completely independent. Therefore,
we can move them into independent methods.
Depends on D79964
Differential Revision: https://phabricator.services.mozilla.com/D79965
I guess that the `Maybe` is `mOffset` of `EditorDOMPointBase` because new
`Maybe::value()`s are called with checking `Maybe::isSome()`. So, accessing
`EditorDOMPointBase::mOffset` newly should cause the assertion.
Then, I found a new caller `IsCharASCIISpace()` which calls `Char()` without
validation here:
https://hg.mozilla.org/mozilla-central/diff/289c293af80b12744b5d35c5b8427ba8d8ebf13e/editor/libeditor/WSRunObject.cpp#l1.383
That could be unset, but I cannot reproduce it, but I succeeded to reproduce
similar assertion hit with using empty text node (see the crashtest). I hope
this fixes the original crash too.
Differential Revision: https://phabricator.services.mozilla.com/D79816
This patch tries to implement Blink-compat white-space normalizer for
`HTMLEditor`.
It's difficult to list up our traditional white-space normalization rules
because `WSRunObject` touches white space sequence only when there is not
acceptable case, e.g., an ASCII white-spaces will be adjacent to another
one, and replaces only unacceptable white-space only. Therefore, whether
white-space sequence may start with either an ASCII white-space or an NBSP.
On the other hand, Blink and WebKit makes white-space sequence always
starts with an NBSP or an ASCII white-space (unfortunately, they behave
differently!). So, for web-compat, we should simulate Blink's behavior
because either behavior is reasonable but Blink have more market share.
This patch simply adds new white-space normalization path for the new one,
and it's switchable with a pref, and still disabled by default.
The other reason why we should do this is, our traditional white-space
normalizer touches the DOM a lot of times per edit action, and the timing
is both before and after touches the DOM tree. Therefore, it's difficult
to compute actual deleting range for `InputEvent.getTargetRanges()` and
touching a lot of times causes running mutation event listeners a lot and
creates a lot of transaction class instances. So, new one have a lot of
merits:
1. Improve web-compat
2. Improve the peformance
3. Improve the security
4. Improve the footprint (but this is now worse then traditional one)
5. Simplify the implementation
The new normalizer is mostly implemented with only 3 `HTMLEditor` methods.
One is `HTMLEditor::DeleteTextAndNormalizeSurroundingWhiteSpaces()`. This is
semi-public method for the edit action handlers. This takes a range with
2 `EditorDOMPoinInText` to delete the range simply. This also replaces
surrounding white-space sequence if necessary. For inserting text case,
this method also handles only white-space normalization when it's called
with collapsed range, i.e., same `EditorDOMPointInText`. This tries to use
`RepaceTextWithTransaction()` as far as possible to reduce creation cost of
transaction classes and the footprint.
Another one is `HTMLEditor::ExtendRangeToDeleteWithNormalizingWhiteSpaces()`.
This tries to extend the given range to normalize surrounding white-spaces.
This is currently not optimized for footprint because this may include
white-spaces which do not need to be replaced. This optimization should be
done before shipping, but for now, enabling `InputEvent.getTargetRanges()` in
Nightly channel is more important. So that it should be done in a follow-up
bug.
The other is `HTMLEditor::GenerateWhitepaceSequence()`. This creates
normalized white-space sequence with surrounding character information.
For keeping this method simple as far as possible, we shouldn't optimize
the range of generation even in follow-ups.
Finally, the white-space sequence is not tested in mochitests, so that we
can enable this new normalizer when we run mochitests under
`editor/libeditor/tests`. However, WPT has some tests. We should keep
them running with current normalizer for checking regression. Instead,
we should enable the pref only for the new WPT added by the previous patch.
Depends on D78655
Differential Revision: https://phabricator.services.mozilla.com/D78656
I realized that there is no word "whitespace" in formal English. This patch
replaces it with "white-space" in comments, and change method names to use
"WhiteSpace".
Depends on D78654
Differential Revision: https://phabricator.services.mozilla.com/D78655
Callers of `WSRunObject::GetASCIIWhitespacesBounds()` may want to scan only
previous or next whitespaces. Therefore, we can split it to save creation
cost of `EditorDOMPointInText`.
Additionally, this makes them scan whitespace sequence in a text node until
hitting its end for avoiding to use expensive API of `WSRunScanner`.
Differential Revision: https://phabricator.services.mozilla.com/D77986
This change was only needed when the previous version of the patch for Bug 1632425 wanted to move things into contextmenu event handler. This patch reverts the change as 1) the suggested behavior never landed and 2) opening context menu in a test can cause conflict with other tests.
Differential Revision: https://phabricator.services.mozilla.com/D78673
Fix of bug 1320229 allowed to paste longer text than `maxlength` attribute of
`<input>` and `<textarea>` because it was thought that the longer text causes
"too long" invalidate state, makes users notified and prevent to submit
form data.
However, according to Bug 1636855 comment 7 (*1), it breaks a major enterprise
web app, SAP, at least because it sends form data without checking validity of
each form data and discards invalid data on server side silently.
According to bug 1636855 comment 24 (*2), one of new behavior's fault is
on Gecko side too. The style of `<input>` element or `<textarea>` element
which has too long text after pasting is changed when it loses focus.
Therefore, users can post the data before they know pasted data is too
long if sending the form data with `Enter` key or something immediately
after pasting (i.e., without moving focus) web apps handle it by themselves.
On the other hand, the original bug report, bug 1320229, should be solved in
the future especially in password field because users may register password
which is cut by `maxlength` silently and they don't use builtin password
manager, only the pasted password is saved, and then, they won't be able to
login as the account. This is really long standing issue of the web forms.
An article (*3) warned this to web developers in 2011. Therefore, we should
keep going advance for solving this issue at least in Nightly channel to get
more feedback from testers and web developers.
1 https://bugzilla.mozilla.org/show_bug.cgi?id=1636855#c7
2 https://bugzilla.mozilla.org/show_bug.cgi?id=1636855#c24
3 https://www.christophermanning.org/writing/dont-use-maxlength-on-password-inputs
Differential Revision: https://phabricator.services.mozilla.com/D78613
According to the telemetry data, almost all web apps in the word do not use
mutation event listeners, but if they are used, editor needs to check
whether modifying node is modified by web apps. The checking cost may not
cheap in some cases. Therefore, we check whether the window has at least one
mutation event listeners or not before non-cheap checking.
However, like actual web apps, most our mochitests and WPT do not add
mutation event listeners. Therefore, such additional checking code is not
tested even though they should work as exactly same as the case when
there are mutation event listeners but they don't touch the DOM.
This patch makes it always return only in debug build for checking the
complicated path.
Differential Revision: https://phabricator.services.mozilla.com/D78173
There's no use case for stateful comparators, so they can be just plain
function pointers.
This is used in some hot places like CSS selector matching.
Differential Revision: https://phabricator.services.mozilla.com/D77084
We were dealing with it correctly when switching display from e.g. block
to inline, or such. But we were not dealing with it when the node was
undisplayed.
Handle it properly, and free one frame bit while at it. We can't really
do this for ManualNAC (the editor resizers) because they request a
reframe explicitly.
Differential Revision: https://phabricator.services.mozilla.com/D76679
In favor of the NativeAnonymous versions which they forward to.
Done automatically with:
rg -l 'IsInAnonymousSubtree' | xargs sed -i 's/IsInAnonymousSubtree/IsInNativeAnonymousSubtree/g'
And removing the function definitions afterwards.
Differential Revision: https://phabricator.services.mozilla.com/D76681
