Bug 895197 - Make `HTMLEditor` not delete last list item when its parent is editing host r=m_kato

Although Blink deletes last list item in the case, it's not reasonable because
the list element should have at least one list item element.  Therefore, this
patch fixes the issue, but creates another incompatible behavior.  See adding
WPT for the detail of new behavior.

Differential Revision: https://phabricator.services.mozilla.com/D167778

editor/libeditor/HTMLEditorDeleteHandler.cpp

@@ -1052,6 +1052,18 @@ class MOZ_STACK_CLASS HTMLEditor::AutoDeleteRangesHandler final {
         HTMLEditor& aHTMLEditor, nsIEditor::EDirection aDirectionAndAmount);
 
    private:
+    /**
+     * MaybeReplaceSubListWithNewListItem() replaces
+     * mEmptyInclusiveAncestorBlockElement with new list item element
+     * (containing <br>) if:
+     * - mEmptyInclusiveAncestorBlockElement is a list element
+     * - The parent of mEmptyInclusiveAncestorBlockElement is a list element
+     * - The parent becomes empty after deletion
+     * If this does not perform the replacement, returns "ignored".
+     */
+    [[nodiscard]] MOZ_CAN_RUN_SCRIPT Result<EditActionResult, nsresult>
+    MaybeReplaceSubListWithNewListItem(HTMLEditor& aHTMLEditor);
+
     /**
      * MaybeInsertBRElementBeforeEmptyListItemElement() inserts a `<br>` element
      * if `mEmptyInclusiveAncestorBlockElement` is a list item element which
@@ -5959,6 +5971,16 @@ Element* HTMLEditor::AutoDeleteRangesHandler::AutoEmptyBlockAncestorDeleter::
          HTMLEditUtils::IsRemovableFromParentNode(*editableBlockElement) &&
          !HTMLEditUtils::IsAnyTableElement(editableBlockElement) &&
          HTMLEditUtils::IsEmptyNode(*editableBlockElement)) {
+    // If the removable empty list item is a child of editing host list element,
+    // we should not delete it.
+    if (HTMLEditUtils::IsListItem(editableBlockElement)) {
+      Element* const parentElement = editableBlockElement->GetParentElement();
+      if (parentElement && HTMLEditUtils::IsAnyListElement(parentElement) &&
+          !HTMLEditUtils::IsRemovableFromParentNode(*parentElement) &&
+          HTMLEditUtils::IsEmptyNode(*parentElement)) {
+        break;
+      }
+    }
     mEmptyInclusiveAncestorBlockElement = editableBlockElement;
     editableBlockElement = HTMLEditUtils::GetAncestorElement(
         *mEmptyInclusiveAncestorBlockElement,
@@ -6184,6 +6206,20 @@ HTMLEditor::AutoDeleteRangesHandler::AutoEmptyBlockAncestorDeleter::Run(
   MOZ_ASSERT(mEmptyInclusiveAncestorBlockElement->GetParentElement());
   MOZ_ASSERT(aHTMLEditor.IsEditActionDataAvailable());
 
+  {
+    Result<EditActionResult, nsresult> result =
+        MaybeReplaceSubListWithNewListItem(aHTMLEditor);
+    if (MOZ_UNLIKELY(result.isErr())) {
+      NS_WARNING(
+          "AutoEmptyBlockAncestorDeleter::MaybeReplaceSubListWithNewListItem() "
+          "failed");
+      return result;
+    }
+    if (result.inspect().Handled()) {
+      return result;
+    }
+  }
+
   if (HTMLEditUtils::IsListItem(mEmptyInclusiveAncestorBlockElement)) {
     Result<RefPtr<Element>, nsresult> result =
         MaybeInsertBRElementBeforeEmptyListItemElement(aHTMLEditor);
@@ -6226,6 +6262,69 @@ HTMLEditor::AutoDeleteRangesHandler::AutoEmptyBlockAncestorDeleter::Run(
   return EditActionResult::HandledResult();
 }
 
+Result<EditActionResult, nsresult> HTMLEditor::AutoDeleteRangesHandler::
+    AutoEmptyBlockAncestorDeleter::MaybeReplaceSubListWithNewListItem(
+        HTMLEditor& aHTMLEditor) {
+  // If we're deleting sublist element and it's the last list item of its parent
+  // list, we should replace it with a list element.
+  if (!HTMLEditUtils::IsAnyListElement(mEmptyInclusiveAncestorBlockElement)) {
+    return EditActionResult::IgnoredResult();
+  }
+  RefPtr<Element> parentElement =
+      mEmptyInclusiveAncestorBlockElement->GetParentElement();
+  if (!parentElement || !HTMLEditUtils::IsAnyListElement(parentElement) ||
+      !HTMLEditUtils::IsEmptyNode(*parentElement)) {
+    return EditActionResult::IgnoredResult();
+  }
+
+  nsCOMPtr<nsINode> nextSibling =
+      mEmptyInclusiveAncestorBlockElement->GetNextSibling();
+  nsresult rv = aHTMLEditor.DeleteNodeWithTransaction(
+      MOZ_KnownLive(*mEmptyInclusiveAncestorBlockElement));
+  if (NS_FAILED(rv)) {
+    NS_WARNING("EditorBase::DeleteNodeWithTransaction() failed");
+    return Err(rv);
+  }
+  Result<CreateElementResult, nsresult> insertListItemResult =
+      aHTMLEditor.CreateAndInsertElement(
+          WithTransaction::Yes,
+          parentElement->IsHTMLElement(nsGkAtoms::dl) ? *nsGkAtoms::dd
+                                                      : *nsGkAtoms::li,
+          !nextSibling || nextSibling->GetParentNode() != parentElement
+              ? EditorDOMPoint::AtEndOf(*parentElement)
+              : EditorDOMPoint(nextSibling),
+          [](HTMLEditor& aHTMLEditor, Element& aNewElement,
+             const EditorDOMPoint& aPointToInsert) -> nsresult {
+            RefPtr<Element> brElement =
+                aHTMLEditor.CreateHTMLContent(nsGkAtoms::br);
+            if (MOZ_UNLIKELY(!brElement)) {
+              NS_WARNING(
+                  "EditorBase::CreateHTMLContent(nsGkAtoms::br) failed, but "
+                  "ignored");
+              return NS_OK;  // Just gives up to insert <br>
+            }
+            IgnoredErrorResult error;
+            aNewElement.AppendChild(*brElement, error);
+            NS_WARNING_ASSERTION(!error.Failed(),
+                                 "nsINode::AppendChild() failed, but ignored");
+            return NS_OK;
+          });
+  if (MOZ_UNLIKELY(insertListItemResult.isErr())) {
+    NS_WARNING("HTMLEditor::CreateAndInsertElement() failed");
+    return insertListItemResult.propagateErr();
+  }
+  CreateElementResult unwrappedInsertListItemResult =
+      insertListItemResult.unwrap();
+  unwrappedInsertListItemResult.IgnoreCaretPointSuggestion();
+  rv = aHTMLEditor.CollapseSelectionTo(
+      EditorRawDOMPoint(unwrappedInsertListItemResult.GetNewNode(), 0u));
+  if (NS_FAILED(rv)) {
+    NS_WARNING("EditorBase::CollapseSelectionTo() failed");
+    return Err(rv);
+  }
+  return EditActionResult::HandledResult();
+}
+
 template <typename EditorDOMRangeType>
 Result<EditorRawDOMRange, nsresult>
 HTMLEditor::AutoDeleteRangesHandler::ExtendOrShrinkRangeToDelete(

testing/web-platform/tests/editing/other/delete-in-last-definition-list-item-when-parent-list-is-editing-host.html

@@ -0,0 +1,88 @@
+<!doctype html>
+<html>
+<head>
+<meta charset="utf-8">
+<meta name="timeout" content="long">
+<meta name="variant" content="?action=Backspace">
+<meta name="variant" content="?action=Delete">
+<title>Delete in last list item should not delete parent list if it's editing host</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<script src="../include/editor-test-utils.js"></script>
+<script>
+"use strict";
+
+const params = new URLSearchParams(location.search.substring(1));
+const backspace = params.get("action") == "Backspace";
+
+addEventListener("load", () => {
+  document.body.innerHTML ="<dl contenteditable></dl>";
+  const editingHost = document.querySelector("dl[contenteditable]");
+  const utils = new EditorTestUtils(editingHost);
+
+  function addPromiseTest(aTest) {
+    promise_test(async () => {
+      editingHost.focus();
+      utils.setupEditingHost(aTest.innerHTML);
+      await (backspace ? utils.sendBackspaceKey() : utils.sendDeleteKey());
+      utils.normalizeStyleAttributeValues();
+      if (Array.isArray(aTest.expectedResult)) {
+        assert_in_array(editingHost.innerHTML, aTest.expectedResult);
+      } else {
+        assert_equals(editingHost.innerHTML, aTest.expectedResult);
+      }
+      assert_equals(
+        document.body.childNodes.length,
+        1,
+        `The editing host should be the only child of <body> (got: "${document.body.innerHTML}")`
+      );
+      assert_equals(
+        document.body.firstChild,
+        editingHost,
+        `The editing host should be the only child of <body> (got: "${document.body.innerHTML}")`
+      );
+    }, `${backspace ? "Backspace" : "Delete"} in "<dl contenteditable>${aTest.innerHTML}</dl>"`);
+  }
+
+  addPromiseTest({
+    innerHTML: "<dt>{}</dt>",
+    expectedResult: ["<dt></dt>", "<dt><br></dt>"],
+  });
+  addPromiseTest({
+    innerHTML: "<dd>{}</dd>",
+    expectedResult: ["<dd></dd>", "<dd><br></dd>"],
+  });
+  addPromiseTest({
+    innerHTML: "<dd><ul><li>{}</li></ul></dd>",
+    expectedResult: ["<dd></dd>", "<dd><br></dd>"],
+  });
+  addPromiseTest({
+    innerHTML: "<dd><ol><li>{}</li></ol></dd>",
+    expectedResult: ["<dd></dd>", "<dd><br></dd>"],
+  });
+  // If only sub-list in the editing host list element, the sub-list should be
+  // replaced with a list item.
+  addPromiseTest({
+    innerHTML: "<ul><li>{}</li></ul>",
+    expectedResult: ["<dd></dd>", "<dd><br></dd>"],
+  });
+  addPromiseTest({
+    innerHTML: "<ol><li>{}</li></ol>",
+    expectedResult: ["<dd></dd>", "<dd><br></dd>"],
+  });
+  addPromiseTest({
+    innerHTML: "<dl><dt>{}</dt></dl>",
+    expectedResult: ["<dd></dd>", "<dd><br></dd>"],
+  });
+  addPromiseTest({
+    innerHTML: "<dl><dd>{}</dd></dl>",
+    expectedResult: ["<dd></dd>", "<dd><br></dd>"],
+  });
+}, {once:true});
+</script>
+</head>
+<body></body>
+</html>

testing/web-platform/tests/editing/other/delete-in-last-list-item-when-parent-list-is-editing-host.html

@@ -0,0 +1,87 @@
+<!doctype html>
+<html>
+<head>
+<meta charset="utf-8">
+<meta name="timeout" content="long">
+<meta name="variant" content="?list=ul&action=Backspace">
+<meta name="variant" content="?list=ul&action=Delete">
+<meta name="variant" content="?list=ol&action=Backspace">
+<meta name="variant" content="?list=ol&action=Delete">
+<title>Delete in last list item should not delete parent list if it's editing host</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<script src="../include/editor-test-utils.js"></script>
+<script>
+"use strict";
+
+const params = new URLSearchParams(location.search.substring(1));
+const backspace = params.get("action") == "Backspace";
+const list = params.get("list");
+
+addEventListener("load", () => {
+  document.body.innerHTML =`<${list} contenteditable></${list}>`;
+  const editingHost = document.querySelector("[contenteditable]");
+  const utils = new EditorTestUtils(editingHost);
+
+  function addPromiseTest(aTest) {
+    promise_test(async () => {
+      editingHost.focus();
+      utils.setupEditingHost(aTest.innerHTML);
+      await (backspace ? utils.sendBackspaceKey() : utils.sendDeleteKey());
+      utils.normalizeStyleAttributeValues();
+      if (Array.isArray(aTest.expectedResult)) {
+        assert_in_array(editingHost.innerHTML, aTest.expectedResult);
+      } else {
+        assert_equals(editingHost.innerHTML, aTest.expectedResult);
+      }
+      assert_equals(
+        document.body.childNodes.length,
+        1,
+        `The editing host should be the only child of <body> (got: "${document.body.innerHTML}")`
+      );
+      assert_equals(
+        document.body.firstChild,
+        editingHost,
+        `The editing host should be the only child of <body> (got: "${document.body.innerHTML}")`
+      );
+    }, `${backspace ? "Backspace" : "Delete"} in "<${list} contenteditable>${aTest.innerHTML}</${list}>"`);
+  }
+
+  addPromiseTest({
+    innerHTML: "<li>{}</li>",
+    expectedResult: ["<li></li>", "<li><br></li>"],
+  });
+  addPromiseTest({
+    innerHTML: "<li><ul><li>{}</li></ul></li>",
+    expectedResult: ["<li></li>", "<li><br></li>"],
+  });
+  addPromiseTest({
+    innerHTML: "<li><ol><li>{}</li></ol></li>",
+    expectedResult: ["<li></li>", "<li><br></li>"],
+  });
+  // If only sub-list in the editing host list element, the sub-list should be
+  // replaced with a list item.
+  addPromiseTest({
+    innerHTML: "<ul><li>{}</li></ul>",
+    expectedResult: ["<li></li>", "<li><br></li>"],
+  });
+  addPromiseTest({
+    innerHTML: "<ol><li>{}</li></ol>",
+    expectedResult: ["<li></li>", "<li><br></li>"],
+  });
+  addPromiseTest({
+    innerHTML: "<dl><dt>{}</dt></dl>",
+    expectedResult: ["<li></li>", "<li><br></li>"],
+  });
+  addPromiseTest({
+    innerHTML: "<dl><dd>{}</dd></dl>",
+    expectedResult: ["<li></li>", "<li><br></li>"],
+  });
+}, {once:true});
+</script>
+</head>
+<body></body>
+</html>