Previously, `Compute*()` methods temporarily updated `Selection` for
emulating the corresponding `Handle*()` method behavior. However,
they have already been fixed as not using `Selection`. Therefore,
we can make
`AutoBlockElementsJoiner::ComputeRangeToDeleteAtOtherBlockBoundary` stop
updating `Selection` only for
initializing `AutoClonedSelectionRangeArray` instance.
Differential Revision: https://phabricator.services.mozilla.com/D301693
When deleting right block completely, we should wrap the right block
into the range to delete completely. So, in that case, we don't need to
use `HandleNonCollapsedRange` which considers whether the `mLeftContent`
and `mRightContent` need to be merged after deleting content in the
range since we always delete the right block as a variation of the
merge. Instead, we can use `DeleteContentInRange` which is simpler and
faster.
Differential Revision: https://phabricator.services.mozilla.com/D288127
Currently, when we delete entire the right block, e.g., when:
```html
<p>abc[</p><p>def]</p>
```
The range will be extended as:
```html
<p>abc[</p><p>def</p>}
```
then, delete the nodes in the range simply.
However, if the right block is a list, e.g.,
```html
<ul><li>abc[<li><ol><li>def]</li></ol></ul>
```
The range won't be extended as:
```html
<ul><li>abc[<li><ol><li>def</li></ol></ul>}
```
instead, the range is extended as:
```html
<ul><li>abc[<li><ol><li>def</li>}</ol></ul>
```
then, delete the nodes in the range. Then, the DOM becomes:
```html
<ul><li>abc[]<li><ol></ol></ul>
```
Finally, the post-processor delete the empty sub-list, the `<ol>`, as
an unnecessary empty node in `HTMLEditor::RemoveEmptyNodesIn()`. I think
this is a hidden bug which is hidden by the strong post-processor.
This patch makes
`AutoDeleteRangesHandler::GetRangeToAvoidDeletingAllListItemsIfSelectingAllOverListElements()`
selects the sub list as the expected one. This is consistent with the
normal paragraph cases so that it becomes easier to maintain.
However, unfortunately, Chrome computes the target ranges as shrunken
into the deepest editing point when the a list will be deleted.
Therefore, this patch changes the behavior only when the class handles
the deletion, but the first fallback fixes the extant bugs so that
there are a lot of new passes under `/input-events`.
With this change, the deleting result becomes compatible with Chrome
and Safari and that's not expected by
`input-events-get-target-ranges-deleting-in-list-items.tentative.html`.
Therefore, this patch fixes the expectation to make all browsers pass
the test and some tests under `/editing/run` which are consistent with
the new expectation starts passing.
Differential Revision: https://phabricator.services.mozilla.com/D286608
Although for making the further processing simpler and faster, we always
delete the following invisible things when modifying the preceding
content. However, `www.icloud.com/notes` users the invisible
preformatted linefeed as anchor for the post-processing of inserting
text. Therefore, this patch makes the inserting content handlers stop
deleting preformatted linefeed. However, for minimizing the risk, this
does not make them stop deleting unnecessary `<br>` elements.
Note that different from padding `<br>` element, the invisible
preformatted linefeed appears as a line break in the flattened text
content for IME. Therefore, preserving the linefeed makes IME not
receive unexpected deletion of a line break. It might be better for IME.
Interestingly, the other browsers delete padding linefeed if it's a
padding for current empty block. It seems that they do it before
inserting content. However, this patch does not do that. Perhaps, we
should write a method to prepare to insert content and share it wider
in a follow up bug (bug 2021218).
Differential Revision: https://phabricator.services.mozilla.com/D286202
This simplifies a lot position about inserting new padding line break
for the last empty line and each behavior is improved by the new API.
Differential Revision: https://phabricator.services.mozilla.com/D284965
This patch makes the delete handler to use the new API so that the
behavior in some edge cases are improved.
Note that the change in
`AutoBlockElementsJoiner::JoinBlockElementsInSameParent()` is required
to avoid new oranges. That make the deleting range contain the right
block entirely if it's empty. Therefore, in the case, the right block
is directly deleted so that its children won't be moved to the left
block before deleted. Therefore, this improves the `getTargetRanges()`
in the case and saves the footprint with avoiding to create unnecessary
transactions.
Differential Revision: https://phabricator.services.mozilla.com/D284964
In the most cases, the scanner users should ignore empty inline
containers so that the behavior should be the default behavior.
Then, the options becomes clearer what the callers can expect.
Differential Revision: https://phabricator.services.mozilla.com/D284963
Perhaps, we should stop using "visible" from the API names because
some API users use the methods while modifying the DOM. Therefore, the
new APIs explain the position of a line break or whether it's
significant or unnecessary from the layout point of view.
This patch adds API to check a line break position which may return
following/preceding block boundary's element and to scan following
block boundary with ignoring unnecessary line break which is done in
some places with calling
`WSRunScanner::ScanInclusiveNextVisibleNodeOrBlockBoundary()` twice.
Additionally, making the extant internal API mainly `HTMLEditUtils`
use the new API to do the correct handling.
Note that this makes `HTMLEditUtils::GetLeafNodeType` return `Ignore`
if the caller does not want invisible things and the node is in
`display:none` element. This change is required to keep passing some
tests whose editing host is `display:inline` and it's followed by
invisible text like `<script>`.
Differential Revision: https://phabricator.services.mozilla.com/D284962
The post processor touches the DOM multiple times and needs to track
all things of the deleted range, moved range, candidate point to put
caret. However, it still forgot to do that when touching the DOM at
first. To prevent this kind of mistakes, this wraps the post processor
into a lambda and makes the lambda track all things once and only flush
the points after touching the DOM each time. This approach is safer
because if forgetting to flush the trackers after touching the DOM,
the following thing will touch or refer a wrong point so that when the
developers add a mutation, the test should fail.
Differential Revision: https://phabricator.services.mozilla.com/D283635
The `AutoTrackDOMPoint` constructor which takes `nsCOMPtr<nsINode>&` and
`uint32_t` is not used anymore. Thus, we can get rid of the mode.
And `FlushAndStopTracking()` should be replaced with
`Flush(StopTracking::Yes)` and align the relatives to use the same
API style.
Differential Revision: https://phabricator.services.mozilla.com/D283634
Similarly, we can implement it with `LeafNodeOptions` to ignore
unnecessary nodes. Then, we can get rid of `WalkTreeOption` and
`WalkTreeOptions` completely.
Differential Revision: https://phabricator.services.mozilla.com/D277524
`WalkTreeOptions` is similar to `LeafNodeOptions` and it's hard to
maintain both of them. Additionally, `LeafNodeOptions` can specify
which type of leaf nodes should be ignored at scanning. Therefore,
`LeafNodeOptions` can be used for them.
Differential Revision: https://phabricator.services.mozilla.com/D277523
`HTMLEditUtils::GetNextContent()` and
`HTMLEditUtils::GetPreviousContent()` are wrappers of
`HTMLEditUtils::GetAdjacentContent()` and
`HTMLEditUtils::GetAdjacentLeafContent()`. The behavior is similar to
`HTMLEditUtils::GetNextLeafContentOrNextBlockElement()` or
`HTMLEditUtils::GetPreviousLeafContentOrPreviousBlockElement()` but
they won't stop at block inclusive ancestor or a block sibling of an
inclusive ancestor. Therefore, we can get rid of them and use the new
and better implementation.
Differential Revision: https://phabricator.services.mozilla.com/D277522
`HTMLEditUtils::Get(Next|Previous)Content()` returns a next/previous
leaf content or next/previous block element if
`WalkTreeOption::StopAtBlockBoundary` is specified. That's the same
as `HTMLEditUtils::Get(Next|Previous)LeafContentOr(Next|Previous)BlockElement()`.
Differential Revision: https://phabricator.services.mozilla.com/D277521
This adds some options which ignore some types of inline elements.
For example, `IgnoreAnyEmptyInlineContainers` is useful to scan previous
or next editable point because Chrome ignores empty inline elements when
deleting from next to an inline element.
`IgnoreInvisibleInlineVoidElements` is useful to scan invisible line
break from end of a block or checking a line break is invisible because
visible inline element makes a line after the last line break. E.g.,
```html
<div>ABC<br><span></span></div>
```
This shows only one line. However,
```html
<div>ABC<br><span style="border:1px solid"></span></div>
```
This shows 2 lines in the `<div>`.
Additionally, renaming the existing `LeafNodeType` values for making
them easier to understand.
Differential Revision: https://phabricator.services.mozilla.com/D277520
An empty inline container element such as `<span></span>` has 2
meanings, if it's visible with specified style, e.g., has non-zero
padding, it should be treated like a replaced element like `<img>`.
However, if it's invisible, it should be treated as `Comment` or
empty `Text`. Therefore, the scanner users need a special handling for
them in some cases. So, it should be identified with the new `WSType`.
Unfortunately, neither
`HTMLEditUtils::IsInclusiveAncestorCSSDisplayNone` nor
`HTMLEditUtils::IsVisibleElementEvenIfLeafNode` is testable within
GTest because it does not have layout information.
Differential Revision: https://phabricator.services.mozilla.com/D277519
This reverts commit 5ccf392dce.
Revert "Bug 1998077 - part 6: Reimplement `HTMLEditUtils::Get(First|Last)Child()` and `HTMLEditUtils::Get(Next|Previous)Sibling()` with `LeafNodeOptions` r=m_kato"
This reverts commit 12eb928cb5.
Revert "Bug 1998077 - part 5: Replace the remaining `HTMLEditUtils::Get(Next|Previous)Content()` with new ones based on `Get(Next|Previous)LeafNodeOr(Next|Previous)BlockElement()` r=m_kato"
This reverts commit 58b4243d10.
Revert "Bug 1998077 - part 4: Replace some callers of `HTMLEditUtils::Get(Next|Previous)Content()` with `HTMLEditUtils::Get(Next|Previous)LeafContentOr(Next|Previous)BlockElement()` r=m_kato"
This reverts commit dd46967eb4.
Revert "Bug 1998077 - part 3: Add new options to ignore empty/invisible leaf nodes r=m_kato"
This reverts commit ca50a4a628.
Revert "Bug 1998077 - part 2: Add `WSType::EmptyInlineContainerElement` r=m_kato"
This reverts commit 4a5d93d631.
Revert "Bug 1998077 - part 1: Make `HTMLEditUtils::IsVisibleElementEvenIfLeafNode` work better if primary frame is available r=m_kato"
This reverts commit b42dc9d35f.
Similarly, we can implement it with `LeafNodeOptions` to ignore
unnecessary nodes. Then, we can get rid of `WalkTreeOption` and
`WalkTreeOptions` completely.
Differential Revision: https://phabricator.services.mozilla.com/D277524
`WalkTreeOptions` is similar to `LeafNodeOptions` and it's hard to
maintain both of them. Additionally, `LeafNodeOptions` can specify
which type of leaf nodes should be ignored at scanning. Therefore,
`LeafNodeOptions` can be used for them.
Differential Revision: https://phabricator.services.mozilla.com/D277523
`HTMLEditUtils::GetNextContent()` and
`HTMLEditUtils::GetPreviousContent()` are wrappers of
`HTMLEditUtils::GetAdjacentContent()` and
`HTMLEditUtils::GetAdjacentLeafContent()`. The behavior is similar to
`HTMLEditUtils::GetNextLeafContentOrNextBlockElement()` or
`HTMLEditUtils::GetPreviousLeafContentOrPreviousBlockElement()` but
they won't stop at block inclusive ancestor or a block sibling of an
inclusive ancestor. Therefore, we can get rid of them and use the new
and better implementation.
Differential Revision: https://phabricator.services.mozilla.com/D277522
`HTMLEditUtils::Get(Next|Previous)Content()` returns a next/previous
leaf content or next/previous block element if
`WalkTreeOption::StopAtBlockBoundary` is specified. That's the same
as `HTMLEditUtils::Get(Next|Previous)LeafContentOr(Next|Previous)BlockElement()`.
Differential Revision: https://phabricator.services.mozilla.com/D277521
This adds some options which ignore some types of inline elements.
For example, `IgnoreAnyEmptyInlineContainers` is useful to scan previous
or next editable point because Chrome ignores empty inline elements when
deleting from next to an inline element.
`IgnoreInvisibleInlineVoidElements` is useful to scan invisible line
break from end of a block or checking a line break is invisible because
visible inline element makes a line after the last line break. E.g.,
```html
<div>ABC<br><span></span></div>
```
This shows only one line. However,
```html
<div>ABC<br><span style="border:1px solid"></span></div>
```
This shows 2 lines in the `<div>`.
Additionally, renaming the existing `LeafNodeType` values for making
them easier to understand.
Differential Revision: https://phabricator.services.mozilla.com/D277520
An empty inline container element such as `<span></span>` has 2
meanings, if it's visible with specified style, e.g., has non-zero
padding, it should be treated like a replaced element like `<img>`.
However, if it's invisible, it should be treated as `Comment` or
empty `Text`. Therefore, the scanner users need a special handling for
them in some cases. So, it should be identified with the new `WSType`.
Unfortunately, neither
`HTMLEditUtils::IsInclusiveAncestorCSSDisplayNone` nor
`HTMLEditUtils::IsVisibleElementEvenIfLeafNode` is testable within
GTest because it does not have layout information.
Differential Revision: https://phabricator.services.mozilla.com/D277519
The remaining case of bug 2005895 is, there is a invisible `Text`
between paragraphs like:
```html
<p>abc</p> <p>{}<br></p>
```
The reason is, `HTMLEditUtils::GetPreviousContent` is not called with
`WalkTreeOption::IgnoreWhiteSpaceOnlyText`. Therefore, I tried to
add the option in `AutoEmptyBlockAncestorDeleter::GetNewCaretPosition`.
However, the handling is odd [1]:
```cpp
static bool IsContentIgnored(const nsIContent& aContent,
const WalkTreeOptions& aOptions) {
```
<snip>
```cpp
if (aOptions.contains(WalkTreeOption::IgnoreWhiteSpaceOnlyText) &&
aContent.IsText() &&
const_cast<Text*>(aContent.AsText())->TextIsOnlyWhitespace()) {
return true;
}
```
So, whether the `Text` is actually visible or not is not tested.
Therefore, this does not work with preformatted content. However, I'd
like to uplift this to ESR140 so that it's too risky to change the
extant `HTMLEditUtils` implementation. Therefore, this patch makes
`GetNewCaretPosition` ignore invisible `Text` nodes directly.
1. https://searchfox.org/firefox-main/rev/20a1fb35a4d5c2f2ea6c865ecebc8e4bee6f86c9/editor/libeditor/HTMLEditUtils.h#3268-3269,3279-3282
Differential Revision: https://phabricator.services.mozilla.com/D277498
The caret should be put before the last line break in a block if there
is no visible content between the line break and the end block boundary.
Additionally,
`WhiteSpaceVisibilityKeeper::DeleteContentNodeAndJoinTextNodesAroundIt()`
should delete the preceding block's invisible `<br>` too.
Differential Revision: https://phabricator.services.mozilla.com/D276756
Currently, we do something special only for `<select>`, `<option>`
and `<optgroup>`. However, we should treat replaced elements and
void elements more consistently.
This patch adds new `HTMLEditUtils` methods to check them easier and
stricter and get the root node of them because DOM API can make invalid
tree even in a void element.
Then, we should handle insertions before the replaced or the void
element if selection range is collapsed in it. However, deletion should
not be handled for that if the range is completely in such element.
Additionally, if range boundary is in such element, we should adjust
the range not to include such element since it's not fully selected
so that shouldn't be deleted nor replaced. These rules are basically
compatible with Chrome.
Finally, if such element is completely hidden, such elements should be
deleted silently with deleting surrounding content if collapsed next to
one. However, this does not work well with a `Backspace` key press even
after applying this patch.
Finally, some tests of `editing-around-select-element.tentative.html`
start failing. The reason is, this patch makes the surrounding block
boundary of `<select>` with adjusting the selection range and that makes
`<select>` moved into another parent. At this time, the `<select>`
gets `style="white-space: nowrap"`. I'm not sure where did this but
not a critical failures. Therefore, this patch contains this
regression.
Differential Revision: https://phabricator.services.mozilla.com/D275170
In theory, the low level APIs of `HTMLEditUtils` should not choose
`BlockInlineCheck` value. However, the callers may want if the
direction is changed in the API, e.g., when scanning siblings but then
climbing up. For preserving the low level API behavior, this patch
adds new `BlockInlineCheck` value, `Auto`. Then, the `HTMLEditUtils`
APIs choose proper value for the direction automatically only when
`Auto` is specified.
Differential Revision: https://phabricator.services.mozilla.com/D270118
It's not required explicitly to specify
`BlockInlineCheck::UseComputedDisplayStyle` and
`BlockInlineCheck::UseComputedDisplayOutsideStyle` to `WSRunScanner`
too. And it should take an `EnumSet` for making it easier to maintain
the class in the future.
Differential Revision: https://phabricator.services.mozilla.com/D270117
In theory, scanning from outside to inside, i.e., checking a sibling
or a child, `display-outside` should be used to check whether the
element is a block or an inline. Similarly, scanning from inside to
outside, i.e., checking a parent, `display-inside` should be used to
check whether the element is a block or an inline.
`HTMLEditUtils`'s related methods are low API, but `WSRunScanner` is
higher API. Therefore, I think `WSRunScanner` should work with the
theory.
This patch makes the internal scanner of `WSRunScanner`,
`TextFragmentData`, take only whether refer HTML default style or CSS
style and consider itself whether use
`BlockInlineCheck::UseComputedDisplayOutsideStyle` or
`BlockInlineCheck::UseComputedDisplayStyle`.
So, this patch should fix some edge case bugs of
`display: inline-block`. However, unfortunately, there is not enough
tests for that.
Differential Revision: https://phabricator.services.mozilla.com/D270116
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
The new behavior is compatible with Chrome. However, if the `<br>` is
immediately after a child block boundary, the result is different from
Chrome. However, I'm not sure whether Chrome's behavior is better
because users won't be able to insert new content where the empty line
was. Therefore, this patch does not fix the latter behavior.
Differential Revision: https://phabricator.services.mozilla.com/D268999
In the historical reason, some of them still take `nsINode*`. In these
days, converting from `nsINode` to `nsIContent` or `Element` is faster
than before and most callers already use `nsIContent`. Therefore, we
should make them take `const nsIContent&`.
Only some methods, allowing callers use pointer makes them simpler.
Therefore, only some methods have overloads which take
`const nsIContent*` instead.
Differential Revision: https://phabricator.services.mozilla.com/D265525
In the historical reason, some of them still take `nsINode*`. In these
days, converting from `nsINode` to `nsIContent` or `Element` is faster
than before and most callers already use `nsIContent`. Therefore, we
should make them take `const nsIContent&`.
Only some methods, allowing callers use pointer makes them simpler.
Therefore, only some methods have overloads which take
`const nsIContent*` instead.
Differential Revision: https://phabricator.services.mozilla.com/D265525
Unfortunately, we need to keep dispatching `devtoolschildremoved` event
to the DevTools. Therefore, this patch renames the `DOMNodeRemoved`
event dispatcher and related methods.
Differential Revision: https://phabricator.services.mozilla.com/D263063
To distinguish whether a DOM modification is expected one or not, we
need to know what the editor is doing and what is expected result by
the mutation.
This patch adds some wrapper classes of DOM API calls and make all
callers of them except the target is very new node which has not been
connected to any DOM.
Differential Revision: https://phabricator.services.mozilla.com/D260587
This reverts commit 73d1ab198c.
Revert "Bug 1973655 - part 2: Make `HTMLEditorMutationObserver.cpp` r=m_kato"
This reverts commit 8603a23c33.
Revert "Bug 1973655 - part 1: Make the editor module not call DOM API to modify the DOM directly r=m_kato"
This reverts commit a8abe8d84c.
Leo Tenenbaum