Commit 95e22b6d authored by Masayuki Nakano's avatar Masayuki Nakano
Browse files

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.

Differential Revision: https://phabricator.services.mozilla.com/D176766
parent b758499f

editor/libeditor/HTMLEditUtils.h

+6 −2
Original line number Diff line number Diff line
@@ -237,6 +237,8 @@ class HTMLEditUtils final { 
                             const nsIContent& aChild) {

    switch (aChild.NodeType()) {

      case nsINode::TEXT_NODE:

      case nsINode::COMMENT_NODE:

      case nsINode::CDATA_SECTION_NODE:

      case nsINode::ELEMENT_NODE:

      case nsINode::DOCUMENT_FRAGMENT_NODE:

        return HTMLEditUtils::CanNodeContain(aParentNodeName,
@@ -246,12 +248,14 @@ class HTMLEditUtils final { 
  }



  // XXX Only this overload does not check the node type.  Therefore, only this

  //     treat Document, Comment, CDATASection, etc.

  //     handle Document and ProcessingInstructionTagName.

  static bool CanNodeContain(nsAtom& aParentNodeName, nsAtom& aChildNodeName) {

    nsHTMLTag childTagEnum;

    // XXX Should this handle #cdata-section too?

    if (&aChildNodeName == nsGkAtoms::textTagName) {

      childTagEnum = eHTMLTag_text;

    } else if (&aChildNodeName == nsGkAtoms::commentTagName ||

               &aChildNodeName == nsGkAtoms::cdataTagName) {

      childTagEnum = eHTMLTag_comment;

    } else {

      childTagEnum = nsHTMLTags::AtomTagToId(&aChildNodeName);

    }

editor/libeditor/HTMLEditor.cpp

+5 −0
Original line number Diff line number Diff line
@@ -2251,6 +2251,11 @@ HTMLEditor::InsertNodeIntoProperAncestorWithTransaction( 
  }

  MOZ_ASSERT(aPointToInsert.IsSetAndValid());



  if (aContentToInsert.NodeType() == nsINode::DOCUMENT_TYPE_NODE ||

      aContentToInsert.NodeType() == nsINode::PROCESSING_INSTRUCTION_NODE) {

    return CreateNodeResultBase<NodeType>::NotHandled();

  }



  // Search up the parent chain to find a suitable container.

  EditorDOMPoint pointToInsert(aPointToInsert);

  MOZ_ASSERT(pointToInsert.IsSet());

editor/libeditor/HTMLEditorDataTransfer.cpp

+15 −0
Original line number Diff line number Diff line
@@ -942,6 +942,9 @@ HTMLEditor::HTMLWithContextInserter::InsertContents( 
              "ignored");

          break;  // from the inner `for` loop

        }

        if (MOZ_UNLIKELY(!moveChildResult.inspect().Handled())) {

          continue;

        }

        inserted = true;

        lastInsertedPoint.Set(child);

        pointToInsert = lastInsertedPoint.NextPoint();
@@ -1026,6 +1029,9 @@ HTMLEditor::HTMLWithContextInserter::InsertContents( 
                "ignored");

            break;  // from the inner `for` loop

          }

          if (MOZ_UNLIKELY(!moveChildResult.inspect().Handled())) {

            continue;

          }

          inserted = true;

          lastInsertedPoint.Set(child);

          pointToInsert = lastInsertedPoint.NextPoint();
@@ -1106,6 +1112,9 @@ HTMLEditor::HTMLWithContextInserter::InsertContents( 
              "ignored");

          break;  // from the inner `for` loop

        }

        if (MOZ_UNLIKELY(!moveChildResult.inspect().Handled())) {

          continue;

        }

        CreateContentResult unwrappedMoveChildResult = moveChildResult.unwrap();

        inserted = true;

        lastInsertedPoint.Set(child);
@@ -1142,6 +1151,9 @@ HTMLEditor::HTMLWithContextInserter::InsertContents( 
              MOZ_KnownLive(content), pointToInsert,

              SplitAtEdges::eDoNotCreateEmptyContainer);

      if (MOZ_LIKELY(moveContentResult.isOk())) {

        if (MOZ_UNLIKELY(!moveContentResult.inspect().Handled())) {

          continue;

        }

        lastInsertedPoint.Set(content);

        pointToInsert = lastInsertedPoint;

        MOZ_ASSERT(pointToInsert.IsSetAndValidInComposedDoc());
@@ -1199,6 +1211,9 @@ HTMLEditor::HTMLWithContextInserter::InsertContents( 
                "ignored");

            continue;  // the inner `for` loop

          }

          if (MOZ_UNLIKELY(!moveParentResult.inspect().Handled())) {

            continue;

          }

          insertedContextParentContent = oldParentContent;

          pointToInsert.Set(oldParentContent);

          MOZ_ASSERT(pointToInsert.IsSetAndValidInComposedDoc());

editor/libeditor/tests/mochitest.ini

+1 −0
Original line number Diff line number Diff line
@@ -292,6 +292,7 @@ skip-if = xorigin # Testing internal API for comm-central 
[test_nsIHTMLEditor_selectElement.html]

[test_nsIHTMLEditor_setBackgroundColor.html]

[test_nsIHTMLObjectResizer_hideResizers.html]

[test_insertHTML_starting_with_multiple_comment_nodes.html]

[test_nsITableEditor_deleteTableCell.html]

[test_nsITableEditor_deleteTableCellContents.html]

[test_nsITableEditor_deleteTableColumn.html]

editor/libeditor/tests/test_insertHTML_starting_with_multiple_comment_nodes.html

0 → 100644
+40 −0
Original line number Diff line number Diff line 
<!doctype html>

<html>

<head>

<meta charset="utf-8">

<title>Insert HTML containing comment nodes</title>

<script src="/tests/SimpleTest/SimpleTest.js"></script>

<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>

<script>

"use strict";



SimpleTest.waitForExplicitFinish();

SimpleTest.waitForFocus(() => {

  const editor = document.querySelector("div[contenteditable]");

  getSelection().collapse(editor.querySelector("li").firstChild, 1);

  document.execCommand("insertHTML", false, "<!-- 1 --><!-- 2 -->b");

  is(

    editor.innerHTML,

    "<ul><li>a<!-- 1 --><!-- 2 -->bc</li></ul>",

    "The HTML fragment should be inserted as-is"

  );

  document.execCommand("undo");

  is(

    editor.innerHTML,

    "<ul><li>ac</li></ul>",

    "Undoing should work as expected"

  );

  document.execCommand("redo");

  is(

    editor.innerHTML,

    "<ul><li>a<!-- 1 --><!-- 2 -->bc</li></ul>",

    "Redoing should work as expected"

  );

  SimpleTest.finish();

});

</script>

</head>

<body>

<div contenteditable><ul><li>ac</li></ul></div>

</body>

</html>