1724650 - (preserved-white-space-editing) WSRunObject.cpp does not handle white-space: pre-*

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P2)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

WSRunObject.cpp checks only whether white-spaces are preformatted or not. This works correctly only in the white-space: pre;.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Now, this bug and bug 1720809 are top of the my queue. I hope no regression reports block my work anymore!

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1724650 - part 1: Make `HTMLEditUtils` scans collapsible white-spaces with referring the style r=m_kato!

This makes it support white-space: pre-line which only preserve linefeed
characters. However, the APIs are not all of what we need to change.
Therefore, this causes hitting assertion hits due to different result from
some other APIs. So patching only with this does not work. The automated
test results will be updated later.

Note that it's not the goal of this bug that we don't support white-space
handling in text nodes whose white-space is -moz-pre-space. It replaces
linefeed characters with white-spaces then, preserve all white-spaces.
However, it's not standardized, and it's available with xml:space="preserve"
in SVG if web developers don't use the vender prefixed value, and finally,
supporting it makes our white-space handling in editor more complicated
than applying the following patches. So, we don't need to do it at least
for now. Therefore, this and following patches does not assume that there
are no cases that only new lines are collapsible.

Depends on D124554

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1724650 - part 2: Make scanner methods in `WSRunObject.cpp` treat preformatted linefeed characters correctly r=m_kato!

Currently, they don't assume that there is white-space: pre-line which
preserves only linefeed characters, but white-spaces are collapsible.
This patch makes them stop early return when white-spaces are preformatted,
and do additional checks for linefeed characters.

Depends on D124555

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1724650 - part 3: Make `GetEndOfCollapsibleASCIIWhiteSpaces` and `GetFirstASCIIWhiteSpacePointCollapsedTo` treat preformatted linefeed characters correctly r=m_kato!

If white-space is pre-line, only linefeed characters are preformatted, but
white-spaces are collapsible. And if collapsible white-spaces follow or
are followed by a preformatted linefeed, they are removed. Therefore,
these methods need to scan following or preceding white-spaces of first
linefeed if the caller wants to delete. Unless doing it, the removed
white-spaces would become visible due to no linefeed character containing
sequence.

Depends on D124556

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1724650 - part 4: Make replace text handlers aware of `white-space: pre-line` r=m_kato!

It currently replaces inserting string's white-spaces to an NBSP even if
preformatted linefeed characters. Additionally, collapsible white-spaces around
a linefeed are removed. Therefore, adjacent collapsible characters of every
linefeed needs to be an NBSP. This patch makes the method handle it correctly.

Depends on D124557

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1724650 - part 5: Make delete text handler aware of `white-space: pre-line` r=m_kato!

If caret is put at preformatted linefeed, we need to handle adjacent
collapsible white-spaces too if the style is white-space: pre-line.
Therefore, this patch makes the delete handler aware of the case only
linefeed characters are preformatted and AutoDeleteRangesHandler call it
when caret is around a preformatted line break.

Depends on D124558

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1724650 - part 6: Make white-space normalizers treat `white-space: pre-line` correctly r=m_kato!

So, the careful point is, they shouldn't put ASCII white-space next to a
preformatted white-space, but should put ASCII white-space next to the NBSP
as far as possible for making line break opportunities.

Depends on D124559

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1724650 - part 7: Make `HTMLEditor` treat preformatted linefeed characters correctly when retrieving a line range r=m_kato!

HTMLEditor tries to collect all children in a line. However, it does not
handle preformatted linefeed characters. Especially when there is a
preformatted linefeed immediately before a block boundary, it becomes invisible.
So, the range of line needs to extend the range correctly similar to the case
if there is an invisible <br> element.

Depends on D124560

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1724650 - part 8: Update WPT result and add new tests to insert mulitple white-spaces r=m_kato!

This does NOT add tests to inserttext command which includes linefeed
characters with other characters. It seems that Blink handles 2 or more
length string is handled as multiple calls for every character. So each
linefeed character is handled as insertparagraph, but Gecko treats the
linefeed characters as-is. We should add the tests later with changing
Gecko's behavior, but for now, we should keep current behavior because
this difference must not be trobule in the wild, but we need to improve
preformatted string handler of Gecko better against reported web-compat
issues.

Depends on D124561

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a98cc0fab376
part 1: Make `HTMLEditUtils` scans collapsible white-spaces with referring the style r=m_kato
https://hg.mozilla.org/integration/autoland/rev/8f4afd607329
part 2: Make scanner methods in `WSRunObject.cpp` treat preformatted linefeed characters correctly r=m_kato
https://hg.mozilla.org/integration/autoland/rev/5285cb2cb2eb
part 3: Make `GetEndOfCollapsibleASCIIWhiteSpaces` and `GetFirstASCIIWhiteSpacePointCollapsedTo` treat preformatted linefeed characters correctly r=m_kato
https://hg.mozilla.org/integration/autoland/rev/6f7be8f40d8c
part 4: Make replace text handlers aware of `white-space: pre-line` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/bfb815566776
part 5: Make delete text handler aware of `white-space: pre-line` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/80cb27e1246a
part 6: Make white-space normalizers treat `white-space: pre-line` correctly r=m_kato
https://hg.mozilla.org/integration/autoland/rev/338e26ff2249
part 7: Make `HTMLEditor` treat preformatted linefeed characters correctly when retrieving a line range r=m_kato
https://hg.mozilla.org/integration/autoland/rev/a26a1ab44995
part 8: Update WPT result and add new tests to insert mulitple white-spaces 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/30500 for changes under testing/web-platform/tests

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/a98cc0fab376
https://hg.mozilla.org/mozilla-central/rev/8f4afd607329
https://hg.mozilla.org/mozilla-central/rev/5285cb2cb2eb
https://hg.mozilla.org/mozilla-central/rev/6f7be8f40d8c
https://hg.mozilla.org/mozilla-central/rev/bfb815566776
https://hg.mozilla.org/mozilla-central/rev/80cb27e1246a
https://hg.mozilla.org/mozilla-central/rev/338e26ff2249
https://hg.mozilla.org/mozilla-central/rev/a26a1ab44995

Comment 13

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

Upstream PR merged by moz-wptsync-bot

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

FYI: This changes white-space handling a lot in preformatted block like <pre>. If there were no regressions, this shouldn't affect to Thunderbird because this change almost only in the case of white-space: pre-line which must not be used in email composer. But this is a big change, and other paths also touched (i.e., white-space:normal, white-space:nowrap and white-space:pre). So if something plaintext mode is broken in Thunderbird recently, it may be a regression of this bug.

Note that bug 1720809 also changes insertLineBreak and insertParagraph behavior of white-space:pre, white-space:pre-wrap and white-space:pre-line, but the new behavior is used only when editing host is an inline or inline-block styled element and whose white-space is pre, pre-wrap or pre-line. The email composer's editing host must be <body> element. So the change shouldn't affect to Thunderbird.