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
