From 1fbe7fc2fcb4c07c52ea82475554dd239acac40e Mon Sep 17 00:00:00 2001
From: Masayuki Nakano <masayuki@d-toybox.com>
Date: Tue, 31 Jan 2023 13:24:08 +0000
Subject: [PATCH] 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  | 99 +++++++++++++++++++
 ...item-when-parent-list-is-editing-host.html | 88 +++++++++++++++++
 ...item-when-parent-list-is-editing-host.html | 87 ++++++++++++++++
 3 files changed, 274 insertions(+)
 create mode 100644 testing/web-platform/tests/editing/other/delete-in-last-definition-list-item-when-parent-list-is-editing-host.html
 create mode 100644 testing/web-platform/tests/editing/other/delete-in-last-list-item-when-parent-list-is-editing-host.html

diff --git a/editor/libeditor/HTMLEditorDeleteHandler.cpp b/editor/libeditor/HTMLEditorDeleteHandler.cpp
index cef5f43d9113..1b3311f3e4d2 100644
--- a/editor/libeditor/HTMLEditorDeleteHandler.cpp
+++ b/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(
diff --git a/testing/web-platform/tests/editing/other/delete-in-last-definition-list-item-when-parent-list-is-editing-host.html b/testing/web-platform/tests/editing/other/delete-in-last-definition-list-item-when-parent-list-is-editing-host.html
new file mode 100644
index 000000000000..e068f9bcd22a
--- /dev/null
+++ b/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>
diff --git a/testing/web-platform/tests/editing/other/delete-in-last-list-item-when-parent-list-is-editing-host.html b/testing/web-platform/tests/editing/other/delete-in-last-list-item-when-parent-list-is-editing-host.html
new file mode 100644
index 000000000000..7d1594393538
--- /dev/null
+++ b/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>