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
`nsINode::InsertBefore()` removes inserting node from a document if it's in
a document including different document. In this case, `UpdateReflectorGlobal`
in BindingUtils.cpp calls `ErrorResult::MightThrowJSException()`, but editor
never throws exception with `ErrorResult`. Therefore, editor needs to call
`ErrorResult::WouldReportJSException()` explicitly if the inserting node may
be in another document.
As far as I checked, it can happen only when undoing or redoing a transaction.
Therefore, this patch touches only transaction classes.
Depends on D81683
Differential Revision: https://phabricator.services.mozilla.com/D81684
Although their callers may want to remove empty text nodes around white-space
sequence, but for now, we should make them not return empty text node because
the former's name means so, and the latter should behave similarly for
consistency.
Differential Revision: https://phabricator.services.mozilla.com/D81683
In most methods of `WSRunScanner`, `WSFragment`s are never used. Therefore,
this patch makes them created when they are necessary.
Depends on D80315
Differential Revision: https://phabricator.services.mozilla.com/D80638
`WSFragment` is created at least one instance, and at most 3 instances per
`WSRunScanner` instance. They can be managed with `AutoTArray` simpler and
we can avoid heap allocation with this approach.
Differential Revision: https://phabricator.services.mozilla.com/D80314
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
It's not used by mozilla-central, comm-central nor BlueGriffon, and it cannot
work with replacing content, mutation event listener's changes. Therefore,
we should remove this for now. If we need to support this **feature** in some
business reasons, we should provide better API than this anyway.
Differential Revision: https://phabricator.services.mozilla.com/D76080
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
For warning the potential of callers' bug, `GetPreviousSiblingOfChild()` outputs
warning into the console when its container is not a container element, e.g.,
it's odd trying to get previous sibling of child if the container is a text
node. Therefore, this patch makes the new caller,
`WSRunScanner::GetPreviousEditableCharPoint()`, check
`CanContainerHaveChildren()` before calling it.
Differential Revision: https://phabricator.services.mozilla.com/D75888
This patch makes it use early-return style and adds some comments. Its behavior
should be changed as soon as possible for web-compat. Therefore, I don't
write current behavior as comment at declaration.
Differential Revision: https://phabricator.services.mozilla.com/D75476
Although the new name is not true when neither
`maybeNBSPFollowingVisibleContent` nor `isPreviousCharASCIIWhitespace` is true.
Differential Revision: https://phabricator.services.mozilla.com/D75475
`WSRunScanner` scans around given point in `GetWSNodes()` at construction with
using `HTMLEditUtils` methods and caches editable text nodes between
`mStartReasonContent` and `mEndReasonContent`. However, it's used only by
`GetNextCharPoint()` and `GetPreviousCharPoint()`, and they shouldn't be
referred after changing the DOM tree. Therefore, we can scan it directly
only when it needs to scan.
The patch rewrites `GetNextCharPoint()` and `GetPreviousCharPoint()` without
`mNodeArray` and removes `mNodeArray` from its member. This may increase the
cost of scanning next/previous text node, but improves the scan performance
when it does not treat so wide range and they are called with a point whose
container is not a text node.
This patch unexpectedly changes the behavior of them, that causes the fix of
2 failures in `insertlinebreak.html` and `insertparagraph.html`. According to
debugger, previously GetNextCharPoint()` in
`ScanNextVisibleNodeOrBlockBoundaryFrom()` called point at `<br>` element
returned no next char, then, `ScanNextVisibleNodeOrBlockBoundaryFrom()` returned
end point which is block boundary of `<listing>` element (it is legacy HTML2
element and treated as `<pre>` element internally). Therefore, the inserted
`<br>` element was misunderstood as invisible `<br>` at end of a block and
inserted another `<br>` element for making it visible. However, the redesigned
one fixed this bug with searching correct text node. Therefore, I cannot
keep the buggy behavior in the new designed methods.
Depends on D75470
Differential Revision: https://phabricator.services.mozilla.com/D75471
`WSRunObject::DeleteRange()` removes only text nodes which are stored when
`WSRunObject` is created. Although it removes text nodes if it's removed,
this patch does not take care about it in the new method. The reason is
the following patch will remove `mNodeArray` and anyway DOM tree modifiers
can check whether they are in proper position before access if it's needed.
Differential Revision: https://phabricator.services.mozilla.com/D75470
The location wasn't used from the caller of
`GetTableCellLocationFromRange`.
However, `GetTableCellLocationFromRange`
included flushing frames, this is now done in
`HTMLEditor::CellIndexes::Update`.
Differential Revision: https://phabricator.services.mozilla.com/D75098
They are returning same meaning value as
`HTMLEditUtils::GetNextLeafContentOrNextBlockElement()` and
`HTMLEditUtils::GetPreviousLeafContentOrPreviousBlockElement()` so that
they can be renamed to same name to overload them.
Depends on D74805
Differential Revision: https://phabricator.services.mozilla.com/D74806
Similar to the previous patch, this patch moves it into `HTMLEditUtils` with
renaming it to `GetPreviousLeafContentOrPreviousBlockElement()`.
Depends on D74804
Differential Revision: https://phabricator.services.mozilla.com/D74805
It looks for next leaf content or next block element for finding "end reason
node" of `WSRunScanner`. Especially for clearing the latter case, this
patch renames it to `GetNextLeafContentOrNextBlockElement()`.
Differential Revision: https://phabricator.services.mozilla.com/D74804
This is used by `TextEditor::UndoAsAction()` too, so, it might be better to be
in `EditorUtils`. However, it is completely designed for `HTMLEditor` and
ideally, `UndoAsAction` should be overridden by `HTMLEditor` too and
`TextEditor` should use faster path because of its content is in anonymous
subtree. But we don't have any merit to do that instead of avoiding virtual
call.
Differential Revision: https://phabricator.services.mozilla.com/D74362
`HTMLEditor` does not remove padding `<br>` element for last empty line when
it becomes unnecessary.
This patch removes it when inserting some text into a text node and it's
followed by a padding <br> element.
Differential Revision: https://phabricator.services.mozilla.com/D74203
I guess that this crash may occur in these conditions:
1. It pointed end of a node, but the node has fewer children at redo.
2. The storing point has already been reset by cycle collector.
So, anyway, the check should be tested only when the point stores a
child node.
This adds WPT tests for related situation. Although "insertLineBreak" of
Blink and WebKit behave odd when selection is collapsed between 2 elements.
Differential Revision: https://phabricator.services.mozilla.com/D73416
Our editor's deletion code removes nodes step-by-step. Therefore, even when
somebodies call `DeleteSelectionWithTransaction()` for removing non-collapsed
ranges, they may have already removed all contents in the range. In such
case, all callers shouldn't call `DeleteSelectionWithTransaction()`.
This makes `test_bug1425997.html` allow to run nexted `execCommand`. It'll be
disabled even in the release channel, but we should keep testing it for
detecting bug of edge cases (like this bug). Note that all crashtests which
test nested `execCommand` calls run with allowing it with the pref for same
reason.
Differential Revision: https://phabricator.services.mozilla.com/D73402
The other `Move*()` are not move nodes without transactions. For making
developers to distinguish the difference, this patch appends `WithTransaction`
postfix like other methods which modify DOM tree with transactions.
Differential Revision: https://phabricator.services.mozilla.com/D72847
Even though the method returns only in specific cases, but the result affects
only one caller, `HTMLEditor::GetNextSelectedTableCellElement()`. Therefore,
we can create new generic utility method,
`HTMLEditUtils::GetTableCellElementIfOnlyOneSelected()` and get rid of
`HTMLEditor::GetCellFromRange()`.
Note that the warnings in `HTMLEditor::GetCellFromRange()` is just noise for
any callers. So, this gets rid of the useless spam warnings.
Differential Revision: https://phabricator.services.mozilla.com/D72586
As an alternative of `HTMLEditor::GetCellFromRange()`, this patch creates more
generic utility method.
Depends on D72296
Differential Revision: https://phabricator.services.mozilla.com/D72585
Currently, `EditorBase::ExtendSelectionForDelete()` depends on some
`nsISelectionController` methods to compute extended range for deletion
from collapsed selection. They are implemented by
`nsFrameSelection::MoveCaret()` and `nsFrameSelection::TakeFocus()`.
Ideally, we should split these methods for computation part and performing
part. However, they change selection with updating other selection state,
for example, table selection state and bidi information. Therefore, it's
impossible to split them with simple code. However, I need to change
`EditorBase::ExtendSelectionForDelete()` just return extended range.
Therefore, this patch creates `nsFrameSelection::PeekOffsetForCaretMove()`
which has the main path in `MoveCaret()` for the `EditorBase` method.
Then, `MoveCaret()` and new `nsFrameSelection::CreateRangeExtendedToSomewhere()`
share the computation code of expanding normal selection.
Finally, this patch wraps `nsFrameSelection::CreateRangeExtendedToSomewhere()`
with new public inline methods for `EditorBase`.
The following patch will remove no-user methods of `nsISelectionController`.
Differential Revision: https://phabricator.services.mozilla.com/D72295
They are used only by `HTMLEditor` so that we should hide them from
`TextEditor` for making it clearer that they are not used by `TextEditor`.
Note that there are 2 `DeleteNodeWithTransaction()` in `HTMLEditor` class.
One is `EditorBase`'s method and the other is `HTMLEditor`'s method.
`HTMLEditor`'s one is check whether the removing node is editable or not,
but in some cases, we need to move non-editable nodes. Therefore, this
patch makes `ReplaceContainerWithTransaction()` call `EditorBase`'s one
for keeping current behavior.
Differential Revision: https://phabricator.services.mozilla.com/D72822
The out params mean the last collapsed selection range's result (although,
the meaning is odd and offset and length are not overwritten when there is
another collapsed range and it causes `DeleteNodeTransaction`). Additionally,
when and only when `DeleteNodeTransaction` and `DeleteTextTransaction` are
added to the `EditAggregationTransaction` created by
`CreateTransactionForSelection()`. Therefore, same result can be looked for
from its only caller, `DeleteSelectionWithTransaction()`.
Note that this makes the method slower if there are too many selection ranges,
but such case must be rare because:
1. We can assume that users rarely use multiple selection ranges for removing
multiple ranges of content except table.
2. Multiple selection is supported only by Gecko. Therefore, web apps must
not use multiple selection for this purpose.
So, it must be okay to use this slower approach for making the methods simpler.
If it'd become damage to some benchmarks, let's create faster access to get
transaction type.
Depends on D72293
Differential Revision: https://phabricator.services.mozilla.com/D72294
`HTMLEditor::DeleteSelectionWithTransaction()` calls `EditorBase`'s overridden
method and handles `nsIEditor::eStrip` case. Therefore, we can rename it with
stop calling the `EditorBase::DeleteSelectionWithTransaction()`, and make it
called by `EditorBase::DeleteSelectionWithTransaction()` only when it's
necessary.
Additionally, we can make all internal method callers of editor classes always
set `nsIEditor::eNoStrip` if the instance is `TextEditor`. This must make
the code easier to understand.
Depends on D72292
Differential Revision: https://phabricator.services.mozilla.com/D72293
The parameter is used only by `EditorBase::CreateTxnForDeleteRange()` to
extend collapsed range, but it accepts only `nsIEditor::eNext` and
`nsIEditor::ePrevious`. Therefore, using `nsIEditor::EDirection` does not
make sense. Instead, they should use new `enum class`,
`HowToHandleCollapsedRange`.
Depends on D72291
Differential Revision: https://phabricator.services.mozilla.com/D72292
`DeleteSelection*()` are members of `TextEditor`, but they are also used by
`HTMLEditor`. Therefore, they and pref cache members for them should be
in `EditorBase` too.
Depends on D71911
Differential Revision: https://phabricator.services.mozilla.com/D72290
Currently, all input (including user pastes) to an input field is truncated to
`maxlength`. This diff disables truncation for user pastes.
When (1) `GetEditAction` is `ePaste`, `ePasteAsQuotation`, `eDrop`, or
`eReplaceText` (ie we are dealing with a paste) and (2) `GetEditActionPrincipal`
is `nullptr` (ie we are dealing with a user edit and not a JS edit), allow a
paste without truncation. That means that, in this case, we will return
`EditActionIgnored` instead of trying to truncate the string.
This behavior is controlled by a new preference `editor.truncate_user_pastes`
which is `false` by default (set in `StaticPrefList.yaml`).
We also modify `editor/libeditor/tests/test_bug603556.html` which currently
expects the output of a paste longer than maxlength to be truncated.
Testing: Created
`editor/libeditor/tests/test_pasting_text_longer_than_maxlength.html` which
checks if a user can paste a password longer than `maxlength` and if the field
is then marked as `tooLong` (this was the original concern of the reporter of
Bug 1320229), and
`editor/libeditor/tests/test_setting_value_longer_than_maxlength_with_setUserInput.html`
which checks if `eReplaceText` has consistent behavior regardless of whether the
field has an associated editor (this test works by calling `setUserInput()`
before and after the element gets focus.) `./mach test editor/libeditor` tests
pass.
Differential Revision: https://phabricator.services.mozilla.com/D71689
It's inherited only by `PlaceholderTransaction` and used only for QI.
Therefore, we can get rid of it.
Additionally, this makes storing `PlaceholderTransaction` and
`CompositionTransaction` in `PlaceholderTransaction` faster and safer with
`WeakPtr`.
Finally, this makes `PlaceholderTransaction` always have non-null name
because it caused a lot of useless warnings in
`EditAggregationTransaction::GetName()` and
`PlaceholderTransaction::GetTxnName()`.
Differential Revision: https://phabricator.services.mozilla.com/D71910
In a lot of places in libeditor, we do nothing if given transaction is not
our edit transaction classes' instance. Therefore, it's better to have
casting virtual method in `nsITransaction` for performance because QI cost
may not be cheap.
Differential Revision: https://phabricator.services.mozilla.com/D71908
Users have much better, easier alternatives, like
DOMWindowUtils.{loadSheetUsingURIString,removeSheet}, which we use to
replace the only caller that exists in mozilla-central (the editor
element, which loads EditorOverride.css).
This allows to clean up the style system and editor. There are other
callers in comm-central, but it seems they can switch to DOMWindowUtils
trivially, as the DOMWindowUtils APIs also use the system principal and
thus they can load any URL.
I'll make sure to give them some time with the migration and/or help
out of course.
Differential Revision: https://phabricator.services.mozilla.com/D71263
When `HTMLEditor` is notified of content changes, it may add a runnable method
`HTMLEditor::OnModifyDocument` or `HTMLEditor::NotifyRootChanged` for each
notification. However, their code do not need running twice nor more. This
could cause performance issues on complicated web apps which sets `innerHTML`
at every key press.
Differential Revision: https://phabricator.services.mozilla.com/D71001
This patch also names the former to `GetInclusiveAncestorBlockElement()` and
the latter to `GetAncestorBlockElement()` for consistency with modern DOM
API names.
Depends on D70882
Differential Revision: https://phabricator.services.mozilla.com/D70883
It's a virtual method which always returns true if `TextEditor`. Therefore,
we can move it into `HTMLEditUtils` and we can make the only caller of
`EditorBase` check `IsTextEditor()` instead.
Depends on D70880
Differential Revision: https://phabricator.services.mozilla.com/D70882
Actually, they are used only by `HTMLEditor` because `TextEditor` finally
returns `true` for any cases in `TextEditor`, but the users are overridden by
`HTMLEditor` and never used by `HTMLEditor`. Therefore, we cam move them
into `HTMLEditUtils`.
Depends on D70878
Differential Revision: https://phabricator.services.mozilla.com/D70879
--HG--
extra : moz-landing-system : lando
Their users should use `EditorDOMPoint` or something instead.
This patch cleans up `EditorBase::DoJoinNodes()` too because of their heavy
user. It requires only joined nodes because `aParent` is used only for
removing `aContentToJoin`, but we now have `nsINode::Remove()` which does
not require parent node and never fails.
Depends on D70877
Differential Revision: https://phabricator.services.mozilla.com/D70878
--HG--
extra : moz-landing-system : lando
A lot of editor code refers `nsINode::IsText()` directly so that we don't
need to keep the too simple editor API anymore.
Depends on D70876
Differential Revision: https://phabricator.services.mozilla.com/D70877
--HG--
extra : moz-landing-system : lando
It's only non-`HTMLEditor` user is `EditorBase::JoinNodesDeepWithTransaction()`,
but it's called only by `HTMLEditor`. Therefore, we can move it into
`HTMLEditUtils` and move `EditorBase::JoinNodesDeepWithTransaction()` to
`HTMLEditor`.
Depends on D70875
Differential Revision: https://phabricator.services.mozilla.com/D70876
--HG--
extra : moz-landing-system : lando
Due to the include hell, `EditorBase.h` cannot include `EditorUtils.h`.
Therefore we need these 3 methods once. Additionally, `IsModifiableNode()`
is really odd method and looks like that it's used for the following 2 purposes:
1. Simply can be editable.
2. Can be removed from parent.
For the former case, we should sort out it with
`EditorUtils::IsEditableContent()`, but for now, this patch moves it to
`HTMLEditUtils::IsSimplyEditable()`. On the other hand, for the latter case,
we obviously has a bug. Therefore, this patch creates
`HTMLEditUtils::IsRemovableFromParentNode()` and make it check whether the
removing node is also editable.
Unfortunately, `EditorUtils::IsEditableContent()` needs to take editor type.
But it's most callers are in `HTMLEditor` and most of remains are in
common methods of `EditorBase`. I guess we could remove this ugly argument
in the future.
Depends on D70874
Differential Revision: https://phabricator.services.mozilla.com/D70875
--HG--
extra : moz-landing-system : lando
`HTMLEditor::IsBlockNode()` is a virtual method derived from
`EditorBase::IsBlockNode()`. For the performance, `EditorBase`'s one just
return false for text node and `<br>` element. However, these check cost
is really cheap in these days. Therefore, we can make `TextEditor` also
use rich API.
This patch moves `HTMLEditor::NodeIsBlockStatic()` and
`HTMLEditor::NodeIsInlineStatic()` to `HTMLEditUtils`, and removes the wrapper
of the fommer, `HTMLEditor::IsBlockNode()` and `WSRunScanner::IsBlockNode()`.
Differential Revision: https://phabricator.services.mozilla.com/D70874
--HG--
extra : moz-landing-system : lando
We can relax about `GetElementOrParentElement*()` because they just return
`nullptr` when selection anchor is a `Document` node.
Additionally, this patch renames the internal APIs to the names similar to
modern DOM API.
Finally, this adds automated test for
`nsIHTMLEditor.getElementOrParentByTagName()`.
Differential Revision: https://phabricator.services.mozilla.com/D70152
--HG--
extra : moz-landing-system : lando
When it refers computed value of style, it may flush pending notifications.
Therefore, they should be marked as `MOZ_CAN_RUN_SCRIPT`.
Differential Revision: https://phabricator.services.mozilla.com/D70151
--HG--
extra : moz-landing-system : lando
Some methods take `StyleType` to work them with specified values or computed
values. This method hides `StyleType` into `CSSEditUtils` and splits the
public methods which took `StyleType` to 2 methods, one is for working with
specified values, the other is for working with computed values.
Additionally, this patch fixes some argument name and type.
Differential Revision: https://phabricator.services.mozilla.com/D70149
--HG--
extra : moz-landing-system : lando
When it refers computed value of style, it may flush pending notifications.
Therefore, they should be marked as `MOZ_CAN_RUN_SCRIPT`.
Differential Revision: https://phabricator.services.mozilla.com/D70151
--HG--
extra : moz-landing-system : lando
Masayuki Nakano
Mirko Brodesser
Sylvestre Ledru
Simon Giesecke
Chris Fronk
Butkovits Atila
Kagami Sascha Rosylight
tkhan
Gijs Kruitbosch
Kris Maglione
Ping Chen
Emilio Cobos Álvarez
Bogdan Tara
Razvan Maries
Olli Pettay
Honza Bambas
Botond Ballo
sanketh
Jonathan Kew
Mark Banner
Cameron McCormack
Makoto Kato