1877513 - Surprising result when deleting <br><br><div> followed by <span> displayed as block

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Francesco]

+++ This bug was initially created as a clone of Bug #1876913+++

As reported in bug 1876913, deleting the first three lines from attachment 9376784 [details] gives a surprising result: The first line of the span is moved outside the span, as can be seen in attachment 9377123 [details].

Note that the HTML was taken from a Thunderbird "plaintext edit" and bug 1876913 comment #1 describes how to reproduce this in Thunderbird.

Affected versions: FF and TB 123 and 124.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

:masayuki, since you are the author of the regressor, bug 1851951, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

It seems that this is not related to the plaintext email mode. I can reproduce this bug with a simple testcase.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1851951

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Hmm, it seems that this is a special case of:

• joining a child right block and a parent left block
• the right block follows empty line created by the <br> element

In this case, we treat the <br> element invisible and we move the first line outside as same as when the right block follows a non-empty line or another block. So, this requires a risky change, but from point of view of HTMLEditor for Firefox, this is not a recent regression, just a web-compat issue. In Thunderbird, the <span> must have been used for avoiding this behavior, but it's now unavailable due to it's treated as a usual block.

Comment 5

[Francesco]

... for Firefox, this is not a recent regression ...

Right, with a div the behaviour already shows in FF 115.

In Thunderbird, the <span> must have been used for avoiding this behavior, but it's now unavailable due to it's treated as a usual block.

Just for your information, TB has always used a <span> to be able to detect the quoted part of a message. display: block; width: 98vw; was added later so the quote could wrap at the window width and not the body width (which is usually 72ch wide causing a "comb" effect).

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

The behavior in the other browsers is, when deleting range starts from start of a line when selection range across block boundaries of right child/descendant blocks, they don't unwrap the first line from the right block, instead, just delete the preceding lines of the right block.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1851951

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Okay, I finished writing the patch for changing the behavior, but I still need to write a lot of new tests. I hope I'll post the patch soon.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 4/27 - 5/6)]

Attachment: Bug 1877513 - Make `HTMLEditor` deletes only preceding lines of right child block if the range starts from start of a line r=m_kato!

Currently, the editor of Gecko always unwraps first line of the right child
block after deleting selected range when the range starts in a parent block
and ends in a child block. This behavior is almost same as the other browsers,
but the other browsers deletes only preceding lines of the right child block
(i.e., without unwrapping the first line of the right child block) if the range
starts from start of a preceding line, for example, when deleting
<div>abc<br>[def<p>g]hi<br>jkl, Gecko moves "hi" to the parent <div>,
but the other browsers keeps it in the child <p>.

For emulating this special handling, we need to touch 2 paths.

One is Backspace when selection is collapsed at start of the child block. In
this case, only when the preceding line is empty, i.e., there are 2 line breaks
(either <br> or \n in white-space: pre-*), the following break should
be deleted, but the child block should not be touched.

The other is, deleting when selection is not collapsed or Delete when
selection is collapsed at immediately before the child block. In the latter
case, HTMLEditor::HandleDeleteSelection extends Selection using
nsFrameSelection. Then, handle it with same path as deleting non-collapsed
range.

The former is handled with HandleDeleteLineBreak and
ComputeRangeToDeleteLineBreak. The latter is handled with
HandleDeleteNonCollapsedRange and ComputeRangeToDeleteNonCollapsedRange.
The new handlers use the ComputeRangeToDelete*. Therefore, beforeinput
reports exactly same range from getTargetRanges. However, existing paths
do not use same approach and this patch makes HandleDeleteNonCollapsedRange
fall it back to HandleDeleteNonCollapsedRange. Therefore, some if checks
in HandleDeleteNonCollapsedRange are ugly, but I have no better idea to
implement this smarter.

Depends on D207689

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67a63b95c31e
Make `HTMLEditor` deletes only preceding lines of right child block if the range starts from start of a line r=m_kato

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/45926 for changes under testing/web-platform/tests

Comment 12

[pstanciu]

Backed out for causing build bustages on HTMLEditorDeleteHandler.cpp

• Backout link
• Push with failures
• Failure Log

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6358e2178e1c
Make `HTMLEditor` deletes only preceding lines of right child block if the range starts from start of a line r=m_kato

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/6358e2178e1c

Comment 16

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot