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
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
`WSRunScanner::ScanPreviousVisibleNodeOrBlockBoundary()` returns the
position **after** the preceding preformatted line break. Therefore,
it needs to use `offset - 1`, but it uses `offset` as-is. So, it should
use `CreateLineBreak<EditorLineBreak>()` instead.
Additionally, then, the position to split may be a middle of a `Text`.
Therefore, `WhiteSpaceVisibilityKeeper::NormalizeWhiteSpacesBefore()`
is not available for the preformatted line break unless it's at start
of a `Text`. Instead, it needs to use
`WhiteSpaceVisibilityKeeper::NormalizeWhiteSpacesToSplitAt()`.
Finally, `WhiteSpaceVisibilityKeeper::NormalizeWhiteSpacesToSplitAt()`
may delete some collapsed white-spaces before the preformatted line
break. Therefore, it needs to track `EditorLineBreak` with
`AutoTrackLineBreak`. (It forgets automatically flushing itself at
destruction. Therefore, this patch fixes it too.)
Differential Revision: https://phabricator.services.mozilla.com/D282358
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
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
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
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
The method touches `Selection` in its `for` loops and that appear in
the reported profile in bug 1997834. Therefore, it should stop touching
the `Selection`, and its only caller should update `Selection` instead.
Additionally, this rewrites entire the method to make it simpler and
not use variables in the wide scopes. That makes it easier to
understand. However, except in some edge cases, this does not change
the behavior.
Differential Revision: https://phabricator.services.mozilla.com/D271362
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 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
These utility methods behave odd in some edge cases and they are not
tested. Therefore, they should be tested with new unit tests and we
should fix the odd behaviors.
Differential Revision: https://phabricator.services.mozilla.com/D265186
`IMEContentObserver` has a cache to avoid to compute start offset of
inserted or removing text every time. However, that works only when
the same mutation occurs of consecutive nodes.
`MoveSiblingsTransaction` uses `nsINode::InsertBefore()` directly to
connected nodes. Therefore, from `IMEContentObserver` point of view,
multiple pairs of removing text and inserting text occur. So,
`IMEContentObserver` cannot use its cache.
Therefore, `MoveSiblingsTransaction` should remove all nodes first.
Then, `IMEContentObserver` can use the cache for deleting text.
Finally, `MoveSiblingsTransaction` should insert all removed nodes to
the DOM. Then, `IMEContentObserver` can use the cache for inserting
text.
Differential Revision: https://phabricator.services.mozilla.com/D259720
This make `AutoInsertParagraphHandler::HandleInHeadingElement`:
* stop splitting the heading element if split at end
* use `SplitParagraphWithTransaction` if splitting the heading element
Then, this also cases the failures because not touching the invisible
`<br>` element in the heading element. Failing with this reason does
not make sense because not touching the heading element saves the
footprint for undo transactions. Therefore, this patch makes the tests
allow the `<br>`s to stay there.
Differential Revision: https://phabricator.services.mozilla.com/D257131
If the right `<dt>`/`<dd>` is empty, the method replaces it with
opposite type element without copying the style. So, we can consider
whether we can just insert an opposite list item before splitting the
given list item.
Note that new failures were passed by a bug of current code. That is,
the method treats invisible `<br>` as visible. So, the cases cause
different result whether there is an invisible `<br>` at end of the
list item even though it does not cause different look for the users.
Inserting opposite type of list item is a Gecko specific behavior.
I think we should stop doing that in Firefox for the compatibility with
the other browsers in a follow up bug.
Differential Revision: https://phabricator.services.mozilla.com/D257130
`SplitParagraphWithTransaction()` does a lot of additional works to
split a paragraph. This should be done when `insertParagraph` is
performed in an list item.
Then, we can drop the following code in `HandleInListItemElement()`:
* Inserting a padding `<br>` into empty left list item
* Copying styles at end of the left `<li>` to the right empty one
* Considering the caret position with scanning the right list item
Differential Revision: https://phabricator.services.mozilla.com/D257129
Unfortunately, I don't have any ideas to separate this change because
the current behavior is broken, it assumes only simple cases. However,
with rewriting the code simpler, we'll get the unexpected passes.
Therefore, this patch has a big change.
The strategy of the new behavior is, first, put a padding `<br>` to next
to the split point if and only if it's immediately before a block
boundary and delete unnecessary existing invisible `<br>` element if
it puts a new `<br>`. Next, put a `<br>` to end of the left paragraph
if and only if the paragraph ends with a block boundary or a line break
or the split occurred at end of the deepest inline element.
Differential Revision: https://phabricator.services.mozilla.com/D257128
The method does not check invisible white-spaces nor `Comment` nodes.
Therefore, it may work differently from the user's point of view in
some edge cases.
Additionally, for making the following patch simpler and safer, the
split point should be outside a non-splittable element like `<img>`.
Differential Revision: https://phabricator.services.mozilla.com/D257127
In the IE/Netscape era, we inserted a <br> in specific cases even if
`insertParagraph` command is being handled. Although this patch makes
the legacy mode slower because of increasing the number of `<br>`
element scanning, we should split it off to make `HandleInParagraph`
simpler to optimize its logic later.
Differential Revision: https://phabricator.services.mozilla.com/D257126
The lambdas were intended for avoiding to increase helper methods of
only a few specific `HTMLEditor` methods. However, now, the methods are
grouped into `AutoInsertParagraphHandler`. Therefore, we can make the
lambdas as its private methods.
Differential Revision: https://phabricator.services.mozilla.com/D255725
For allowing to create helper methods and share more data between
helper methods without adding a lot of arguments to each method,
they should be stack only class.
Differential Revision: https://phabricator.services.mozilla.com/D255724
Git does not track file copies but Git may keep the per line history if
they detect a file copy. So, this might help to Git the per line
history of insert paragraph command handler and insert line break
command handler.
Differential Revision: https://phabricator.services.mozilla.com/D255723
Leo Tenenbaum