1851951 - Make `insertParagraph` and `delete`/`forwardDelete` should check the style when looking for a block ancestor

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

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

Blink and WebKit ignores ancestor block elements which are styled as inline at handling insertParagraph. Similarly, they do same thing at getting 2 blocks which should be joined by delete or forwardDelete.

We cannot emulate their behavior completely because they may behave odd in some cases at delete/forwardDelete.

E.g., forwardDelete at <div><p style="display:inline">abc[]</p><p style="display:inline">def</p></div>, the result is <div><p style="display:inline">abc[]</p><p style="display:inline">ef</p></div>. That removes following character in another <p>, but caret stays in the first paragraph.

However, for better compatibility, we should make it work in the basic cases.

Comment 1

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

If I just make HTMLEditUtils::IsBlockElement() refer the style, I got:

Unexpected Results
------------------
/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertParagraph
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: div)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline">[]abc</div> (defaultparagraphseparator: div)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline">a[]bc</div> (defaultparagraphseparator: div)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: div) (preserving temporary inline style test)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline"><b>abc[]</b></div> (defaultparagraphseparator: div) (preserving inline style test)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block">abc[]</div> (defaultparagraphseparator: div)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block">[]abc</div> (defaultparagraphseparator: div)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block">a[]bc</div> (defaultparagraphseparator: div)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block">abc[]</div> (defaultparagraphseparator: div) (preserving temporary inline style test)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block"><b>abc[]</b></div> (defaultparagraphseparator: div) (preserving inline style test)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: p)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline">[]abc</div> (defaultparagraphseparator: p)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline">a[]bc</div> (defaultparagraphseparator: p)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: p) (preserving temporary inline style test)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline"><b>abc[]</b></div> (defaultparagraphseparator: p) (preserving inline style test)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block">abc[]</div> (defaultparagraphseparator: p)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block">[]abc</div> (defaultparagraphseparator: p)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block">a[]bc</div> (defaultparagraphseparator: p)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block">abc[]</div> (defaultparagraphseparator: p) (preserving temporary inline style test)
  UNEXPECTED-PASS <div contenteditable style="white-space:nowrap; display:inline-block"><b>abc[]</b></div> (defaultparagraphseparator: p) (preserving inline style test)
/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText
  FAIL <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: div) - assert_equals: A <br> should be inserted at end expected "abc<br><br>" but got "abc<br>"   
@http://web-platform.test:8000/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText:67:22
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2599:25
test@http://web-platform.test:8000/resources/testharness.js:628:30
@http://web-platform.test:8000/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText:41:9
  FAIL <div contenteditable style="white-space:nowrap; display:inline-block">abc[]</div> (defaultparagraphseparator: div) - assert_equals: A <br> should be inserted at end expected "abc<br><br>" but got "abc<br>"
@http://web-platform.test:8000/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText:67:22
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2599:25
test@http://web-platform.test:8000/resources/testharness.js:628:30
@http://web-platform.test:8000/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText:41:9
  FAIL <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: p) - assert_equals: A <br> should be inserted at end expected "abc<br><br>" but got "abc<br>"     
@http://web-platform.test:8000/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText:67:22
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2599:25
test@http://web-platform.test:8000/resources/testharness.js:628:30
@http://web-platform.test:8000/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText:41:9
  FAIL <div contenteditable style="white-space:nowrap; display:inline-block">abc[]</div> (defaultparagraphseparator: p) - assert_equals: A <br> should be inserted at end expected "abc<br><br>" but got "abc<br>"
@http://web-platform.test:8000/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText:67:22
Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:2599:25
test@http://web-platform.test:8000/resources/testharness.js:628:30
@http://web-platform.test:8000/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText:41:9
/editing/run/delete.html?6001-7000
  UNEXPECTED-PASS [["delete",""]] "<p style=display:inline>fo[o<p style=display:inline>b]ar" compare innerHTML

However, this affects too wide area of HTMLEditor, so, I'm checking each caller now.

Comment 2

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

Attachment: Bug 1851951 - Make `HTMLEditor` refer computed `display` instead of the HTML default style at considering block or inline element r=m_kato!

Blink [1] and WebKit [2] refers display-outside value when they consider whether an
element is a block or an inline.

However, our editor refers HTML default style instead. Therefore, our editor
cannot handle the following cases:

• If a container block is a non-HTML element whose display style is block.
• If a container <div> etc is styled as inline and typing Enter in it.
• If a container <span> is styled as block and it ends with spaces.

For making users get better result, we should follow the other browsers.

However, this is too risky change. Therefore, this patch enables new behavior
only in the Nightly channel and early beta builds to collect feedback from
testers.

The big rules of checking block vs. inline are:

• When we handle block level edit actions such as formatting block, indenting
  or outdenting selection, making or removing list, we should keep referring
  the HTML default style because the other browsers do so and the commands are
  intended for modifying the HTML structure.
• Otherwise, we should refer the computed style of the block. However, if
  working with non-connected elements, we may need a special handling that is
  falling back to refer the HTML default style because HTMLEditorDataTransfer
  work with non-connected document fragments.
• Finally, if we check visibility of collapsible white-spaces and <br>s, we
  should refer computed style. However, in this case, we may need to treat
  ancestor inline-blocks as block too, but for siblings, we should refer only
  display-outside.

1. https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/editing/editing_utilities.cc;l=779;drc=31fb07c05718d671d96c227855bfe97af9e3fb20
2. https://searchfox.org/wubkat/rev/b054636fffeffa0314914a11ce39725a3057131e/Source/WebCore/editing/Editing.cpp#317

Comment 3

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

Attachment: Bug 1851951 - Make `HTMLEditUtils::GetElementOfImmediateBlockBoundary` ignore invisible text node r=m_kato!

Without this patch, the following test newly fails:

/editing/other/insertparagraph-with-white-space-style.tentative.html?white-space=nowrap&command=insertText
  FAIL <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: div) - assert_equals: A <br> should be inserted at end expected "abc<br><br>" but got "abc<br>"
  FAIL <div contenteditable style="white-space:nowrap; display:inline">abc[]</div> (defaultparagraphseparator: p) - assert_equals: A <br> should be inserted at end expected "abc<br><br>" but got "abc<br>"

The reasons is, the inlined editing host is at end of the <body>, therefore,
even though the editing host itself is inline, it needs a padding <br> to
make it the new line visible.

However, HTMLEditUtils::GetElementOfImmediateBlockBoundary does not return
<body> because the editing host is followed by <script> which has
a text node which is not white-spaces only. Therefore,
HTMLEditUtils::IsVisibleBRElement considers the <br> element at end of the
editing host is "visible".

Depends on D188598

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/554d63dad12e
Make `HTMLEditor` refer computed `display` instead of the HTML default style at considering block or inline element r=m_kato
https://hg.mozilla.org/integration/autoland/rev/81c485d2c157
Make `HTMLEditUtils::GetElementOfImmediateBlockBoundary` ignore invisible text node r=m_kato

Comment 5

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

Intent to change: https://groups.google.com/a/mozilla.org/g/dev-platform/c/YsAgwshXXms

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/554d63dad12e
https://hg.mozilla.org/mozilla-central/rev/81c485d2c157

Comment 7

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

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

Comment 8

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

Upstream PR merged by moz-wptsync-bot