1639287 - Undoing paragraph deletion incorrectly adds an empty paragraph

Status: RESOLVED FIXED

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

Description

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: paragraph-deletion.html

1. Open the attachment
2. Try deleting in either direction and undoing that action.

Expected: Paragraph should restore
Actual: An empty paragraph appears instead of the original one

Comment 1

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: paragraph-deletion.html

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: paragraph-deletion.html

Comment 3

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: paragraph-deletion.html

Comment 4

[Kagami Rosylight [:saschanaz] (they/them)]

Emojis are unexpectedly broken, filed Bug 1639311.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

I couldn't reproduce this on Win10, nightly 78.

Comment 6

[Kagami Rosylight [:saschanaz] (they/them)]

"Here" is meant to be the right side (or left in the second case) of the emoji, while it's currently broken. Could you try again?

Comment 7

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: paragraph-deletion.html

Trying again after Bug 1639311...

Comment 8

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Still broken; also on Ubuntu.

Comment 9

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

A preparation patch for bug 1735608 will fix this. I'll post the patch to here soon.

Comment 11

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

Attachment: Bug 1639287 - Add WPT to check undo/redo behavior r=m_kato!

Depends on D132120

Comment 12

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

Attachment: Bug 1639287 - part 1: Make `JoinNodeTransaction::UndoTransaction()` use `HTMLEditor::DoSplitNode()` r=m_kato!

JoinNodeTransaction::UndoTransaction() has its own splitting code. However,
it has some bugs, it does not handle surrogate pairs correctly and it does not
care selections. HTMLEditor::DoSplitNode() is used for splitting a DOM node
from SplitNodeTransaction::DoTransaction(). So that we should make
JoinNodeTransaction::UndoTransaction() should use HTMLEditor::DoSplitNode()
for saving the maintenance cost.

Depends on D132121

Comment 13

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

Attachment: Bug 1639287 - part 2: Make `SplitNodeTransaction::RedoTransaction()` use `HTMLEditor::DoSplitNode()` r=m_kato!

Similarly, SplitNodeTransaction::RedoTransaction() has its own code, but it
does not work well. Let's make it use HTMLEditor::DoSplitNode() instead.

This fixes bug 1740656.

Depends on D132122

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ccf78c106ecf
Add WPT to check undo/redo behavior r=m_kato
https://hg.mozilla.org/integration/autoland/rev/0b4c7957b844
part 1: Make `JoinNodeTransaction::UndoTransaction()` use `HTMLEditor::DoSplitNode()` r=m_kato
https://hg.mozilla.org/integration/autoland/rev/517f1f870d28
part 2: Make `SplitNodeTransaction::RedoTransaction()` use `HTMLEditor::DoSplitNode()` r=m_kato

Comment 15

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

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

Comment 16

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/ccf78c106ecf
https://hg.mozilla.org/mozilla-central/rev/0b4c7957b844
https://hg.mozilla.org/mozilla-central/rev/517f1f870d28

Comment 17

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

Upstream PR merged by moz-wptsync-bot