1828078 - Pasting formatted text into a list and then undoing the paste operation breaks the editor

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Teal Dulcet [:tdulcet] (TB Council)]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/111.0

Steps to reproduce:

1. Open the Thunderbird compose window and add a list (either numbered or bulleted) with several items.
2. Copy some formatted text from an external application and paste it into one of the list items.
3. Undo the paste operation (Ctrl + U).

Actual results:

The entire list item is deleted, including the existing content. In addition, both undo and redo quit working, so it is impossible to recover the lost information, resulting in a data loss if it was not captured in the last auto save.

Tested on both Thunderbird ESR 102 and Daily 114.

Expected results:

Only the pasted text should be removed and undo/redo should continue to work as expected.

Comment 1

[MM]

Can you reproduce this in Firefox? For example at https://www-archive.mozilla.org/editor/midasdemo/. It's likely a bug in the Mozilla editor.

Comment 2

[Teal Dulcet [:tdulcet] (TB Council)]

Yes, indeed it is a bug in the Mozilla editor. I thought this could be the case, but I was not sure how to go about testing it, so thanks for that link.

Comment 3

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

I tried to reproduce this bug with the editor in comment 1. However, I cannot reproduce it in Nightly.

1. Do you reproduce it with "Use CSS" enabled in the demo?
2. Which application do you copy the text from?
3. Which list item of the multiline list? First one, last one, a middle one or any ones?
4. Do you copy multiple lines or a single line?
5. Why Ctrl-U rather than Ctrl-Z? Localized version?

Comment 4

[Teal Dulcet [:tdulcet] (TB Council)]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away 5/1-5/5)) from comment #3)

1. Do you reproduce it with "Use CSS" enabled in the demo?

Yes, I can reproduce it both with and without that "Use CSS" option.

2. Which application do you copy the text from?

I just tested it with several applications and so far, I have only been able to reproduce the issue with Microsoft Word. However, MS Word copies the text in many different formats, but the Mozilla editor is of course only using the standard HTML format, so presumably this issue could effect other applications as well.

3. Which list item of the multiline list? First one, last one, a middle one or any ones?

Any of the items. With just one item, the entire list is deleted.

4. Do you copy multiple lines or a single line?

I was copying just a single line, but multiple lines causes the issue as well.

5. Why Ctrl-U rather than Ctrl-Z? Localized version?

Sorry, I meant the standard Ctrl + Z, that was a typo. I am using the default en-US locale.

For clarity, this only reproduces if one pastes the text with the formatting. If one uses Ctrl + Shift + V to paste unformatted, it works as expected. This is actually is what I was trying to do, but I forgot to hold down Shift, so I of course tried to undo the paste operation, but that is when I encountered this bug. The item where I had pasted the text had a whole paragraph worth of information, which was completely lost.

Comment 5

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

Thank you! I reproduced this if I copy text from Word.

Comment 6

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

This does not depend on editor.join_split_direction.compatible_with_the_other_browsers so that this does not block to ship it.

Comment 7

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

There are some problems:

1. Pasting text inserts a line break before the pasted things.
2. Undoing after pasting a single line fails to restore and removes the item pasted things in.
3. Oddly, unexpected last list item deletion also removes the parent list element. This means that it seems that the list item deletion is handled by the delete handler.

Comment 8

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

[Child 53500, Main Thread] WARNING: There was no proper container element to insert the content node in the editing host: file M:/src/editor/libeditor/HTMLEditor.cpp:2279
[Child 53500, Main Thread] WARNING: HTMLEditor::InsertNodeIntoProperAncestorWithTransaction(SplitAtEdges::eDoNotCreateEmptyContainer) failed, but ignored: file M:/src/editor/libeditor/HTMLEditorDataTransfer.cpp:1165
[Child 53500, Main Thread] WARNING: The child is nullptr or doesn't have its parent: 'IsSet()', file M:/fx64-dbg/dist/include\mozilla/EditorDOMPoint.h:156
[Child 53500, Main Thread] WARNING: The child is nullptr or doesn't have its parent: 'IsSet()', file M:/fx64-dbg/dist/include\mozilla/EditorDOMPoint.h:156
[Child 53500, Main Thread] WARNING: '!mParent', file M:/fx64-dbg/dist/include\mozilla/EditorDOMPoint.h:725
[Child 53500, Main Thread] WARNING: Failed to set endPoint to next to aNode: 'advanced', file M:/src/editor/libeditor/EditorBase.cpp:6454
[Child 53500, Main Thread] WARNING: '!aStart.IsSet()', file M:/src/editor/libeditor/EditorBase.cpp:6474
[Child 53500, Main Thread] WARNING: TopLevelEditSubActionData::AddRangeToChangedRange() failed: 'NS_SUCCEEDED(rv)', file M:/src/editor/libeditor/EditorBase.cpp:6458
[Child 53500, Main Thread] WARNING: TopLevelEditSubActionData::AddNodeToChangedRange() failed, but ignored: 'NS_SUCCEEDED(rvIgnored)', file M:/src/editor/libeditor/EditorBase.cpp:6568
[Child 53500, Main Thread] WARNING: '!parentNode', file M:/fx64-dbg/dist/include\mozilla/EditorDOMPoint.h:634
[Child 53500, Main Thread] WARNING: '!aContentToInsert.GetParentNode()', file M:/src/editor/libeditor/HTMLEditor.cpp:2305
[Child 53500, Main Thread] WARNING: EditorBase::InsertNodeWithTransaction() succeeded, but the inserted node was moved or removed by the web app: file M:/src/editor/libeditor/HTMLEditor.cpp:2310
[Child 53500, Main Thread] WARNING: There was no proper container element to insert the content node in the editing host: file M:/src/editor/libeditor/HTMLEditor.cpp:2279
[Child 53500, Main Thread] WARNING: HTMLEditor::InsertNodeIntoProperAncestorWithTransaction(SplitAtEdges::eDoNotCreateEmptyContainer) failed, but ignored: file M:/src/editor/libeditor/HTMLEditorDataTransfer.cpp:1165
[Child 53500, Main Thread] WARNING: mOffset and mChild are mismatched: '!mOffset.isSome() || mParent->GetChildAt_Deprecated( mOffset.value()) == mChild', file M:/fx64-dbg/dist/include\mozilla/EditorDOMPoint.h:886
[Child 53500, Main Thread] WARNING: mOffset and mChild are mismatched: '!mOffset.isSome() || mParent->GetChildAt_Deprecated( mOffset.value()) == mChild', file M:/fx64-dbg/dist/include\mozilla/EditorDOMPoint.h:915
[Child 53500, Main Thread] WARNING: There was no proper container element to insert the content node in the editing host: file M:/src/editor/libeditor/HTMLEditor.cpp:2279
[Child 53500, Main Thread] WARNING: HTMLEditor::InsertNodeIntoProperAncestorWithTransaction(SplitAtEdges::eDoNotCreateEmptyContainer) failed, but ignored: file M:/src/editor/libeditor/HTMLEditorDataTransfer.cpp:1165

Comment 9

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

And at undoing:

[Child 53500, Main Thread] WARNING: EditAggregateTransaction::UndoTransaction() failed: file M:/src/editor/libeditor/PlaceholderTransaction.cpp:81
[Child 53500, Main Thread] WARNING: TransactionItem::UndoTransaction() failed: file M:/src/editor/txmgr/TransactionItem.cpp:107
[Child 53500, Main Thread] WARNING: TransactionItem::UndoTransaction() failed: 'NS_SUCCEEDED(rv)', file M:/src/editor/txmgr/TransactionManager.cpp:113
[Child 53500, Main Thread] WARNING: TransactionManager::Undo() failed: file M:/src/editor/libeditor/EditorBase.cpp:1041
[Child 53500, Main Thread] WARNING: '!atCaret.IsSet()', file M:/src/editor/libeditor/HTMLEditSubActionHandler.cpp:10524
[Child 53500, Main Thread] WARNING: HTMLEditor::EnsureSelectionInBodyOrDocumentElement() failed, but ignored: 'NS_SUCCEEDED(rv)', file M:/src/editor/libeditor/HTMLEditSubActionHandler.cpp:384
[Child 53500, Main Thread] WARNING: '!aAnchorNode', file M:/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:132
[Child 53500, Main Thread] WARNING: 'res.isErr()', file M:/src/extensions/spellcheck/src/mozInlineSpellChecker.cpp:878
[Child 53500, Main Thread] WARNING: mozInlineSpellChecker::SpellCheckAfterEditorChange() failed: 'NS_SUCCEEDED(rv)', file M:/src/editor/libeditor/EditorBase.cpp:5143
[Child 53500, Main Thread] WARNING: EditorBase::HandleInlineSpellCheck() failed: file M:/src/editor/libeditor/HTMLEditSubActionHandler.cpp:725
[Child 53500, Main Thread] WARNING: HTMLEditor::OnEndHandlingTopLevelEditSubActionInternal() failied: 'NS_SUCCEEDED(rv)', file M:/src/editor/libeditor/HTMLEditSubActionHandler.cpp:340
[Child 53500, Main Thread] WARNING: '!content', file M:/src/editor/libeditor/HTMLEditor.cpp:6803
[Child 53500, Main Thread] WARNING: '!targetElement', file M:/src/editor/libeditor/EditorBase.cpp:2286
[Child 53500, Main Thread] WARNING: '!content', file M:/src/editor/libeditor/HTMLEditor.cpp:6803
[Child 53500, Main Thread] WARNING: '!content', file M:/src/editor/libeditor/HTMLEditor.cpp:6803
[Child 53500, Main Thread] WARNING: '!content', file M:/src/editor/libeditor/HTMLEditor.cpp:6803
[Child 53500, Main Thread] WARNING: '!content', file M:/src/editor/libeditor/HTMLEditor.cpp:6803

Comment 10

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

Attachment: Bug 1828078 - part 1: Make `HTMLEditUtils::CanNodeContain` handle comment node r=m_kato!

HTMLEditUtils::CanNodeContain does not handle comment nodes and cdata section
nodes (the latter one is available only in XHTML documents, it's treated as a
comment node in HTML documents).

When copying HTML from Word on Windows, that contains 2 comment nodes at
start of pasting body (which does not appear in clipboard viewer, so, Gecko
creates them somewhere) and that causes HTMLEditUtils::CanNodeContain returns
false for any parents. Therefore,
HTMLEditor::InsertNodeIntoProperAncestorWithTransaction returns error
and the pasting fails with odd state and unexpectedly split the list item in
HTMLWithContextInserter::InsertContents. Finally, undoing fails to do some
of them and causes destroying the editable nodes.

This patch makes HTMLEditUtils::CanNodeContain work with comment nodes
and cdata section nodes (the latter is treated as a comment node since there
is no "cdata" tag definition of nsHTMLTag) and
HTMLEditor::InsertNodeIntoProperAncestorWithTransaction just return
"not handled" result for some other types of nodes which cannot be inserted
in any elements.

Note that the result of pasting from Word is different from Chrome's result.
Chrome does not paste such comment nodes (but inserts comment nodes with
insertHTML command). For now, I don't want to work on fixing this
compatibility issue since comment nodes does not cause any known troubles.
Therefore, this patch does not contain WPT updates.

Comment 11

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

Attachment: Bug 1828078 - part 2: Stop moving comment nodes when deleting a block/line boundary r=m_kato!

With the preceding patch, comment nodes are also moved at deleting a block/line
boundary. However, this causes some WPT failures. Therefore, this adds an
option to the related methods.

Note that Chrome removes all comment nodes in moving nodes. However, I don't
have the motivation to do that because it requires additional cost and I have
no idea to improve the compatibility in usual web apps. So I believe that
doing it wastes the runtime performance unless we'd get a bug reports by the
difference.

Therefore, this patch does not update WPTs too.

Depends on D176766

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac6d2f978e31
part 1: Make `HTMLEditUtils::CanNodeContain` handle comment node r=m_kato
https://hg.mozilla.org/integration/autoland/rev/ef4107e0cf59
part 2: Stop moving comment nodes when deleting a block/line boundary r=m_kato

Comment 13

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/ac6d2f978e31
https://hg.mozilla.org/mozilla-central/rev/ef4107e0cf59

Comment 14

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

The patch landed in nightly and beta is affected.
:masayuki, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox114 to wontfix.

For more information, please visit BugBot documentation.

Comment 15

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

This is an old bug and this changes behavior a little. So I think that we should not uplift this to beta unless drivers want to do it.