Bug 1923250 - Make HTMLEditor::InsertElementAtSelectionAsAction split ancestor inline elements r=m_kato

Chrome and Safari splits ancestors when `document.execCommand("insertImage")` inserts an `<img>`, but we insert into the closest inline element. For example, ```html <b>A[]B</b> ``` Chrome and Safari make it to: ```html <b>A</b><img><b>B</b> ``` But Firefox makes it to: ```html <b>A<img>B</b> ``` I think that we should not change the behavior on Thunderbird. Therefore, the behavior is controlled with the new `options` argument and the new behavior runs only when the `HTMLEditor` works for content document and it's not caused by the XPCOM method. Differential Revision: https://phabricator.services.mozilla.com/D225037
2024-11-23 04:41:11 +00:00 · 2024-10-11 22:46:28 +00:00 · 2024-10-11 22:46:28 +00:00 · b3b969af35
commit b3b969af35
parent 1a4dbacb0f
6 changed files with 115 additions and 33 deletions
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@ -2088,14 +2088,19 @@ NS_IMETHODIMP HTMLEditor::RebuildDocumentFromSource(

 NS_IMETHODIMP HTMLEditor::InsertElementAtSelection(Element* aElement,
                                                   bool aDeleteSelection) {
-  nsresult rv = InsertElementAtSelectionAsAction(aElement, aDeleteSelection);
+  InsertElementOptions options;
+  if (aDeleteSelection) {
+    options += InsertElementOption::DeleteSelection;
+  }
+  nsresult rv = InsertElementAtSelectionAsAction(aElement, options);
  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                       "HTMLEditor::InsertElementAtSelectionAsAction() failed");
  return rv;
 }

 nsresult HTMLEditor::InsertElementAtSelectionAsAction(
-    Element* aElement, bool aDeleteSelection, nsIPrincipal* aPrincipal) {
+    Element* aElement, const InsertElementOptions aOptions,
+    nsIPrincipal* aPrincipal) {
  if (NS_WARN_IF(!aElement)) {
    return NS_ERROR_INVALID_ARG;
  }
@ -2175,14 +2180,19 @@ nsresult HTMLEditor::InsertElementAtSelectionAsAction(
    }
  }

-  if (aDeleteSelection) {
+  if (aOptions.contains(InsertElementOption::DeleteSelection) &&
+      !SelectionRef().IsCollapsed()) {
    if (!HTMLEditUtils::IsBlockElement(
            *aElement, BlockInlineCheck::UseComputedDisplayOutsideStyle)) {
      // E.g., inserting an image.  In this case we don't need to delete any
      // inline wrappers before we do the insertion.  Otherwise we let
      // DeleteSelectionAndPrepareToCreateNode do the deletion for us, which
      // calls DeleteSelection with aStripWrappers = eStrip.
-      nsresult rv = DeleteSelectionAsSubAction(eNone, eNoStrip);
+      nsresult rv = DeleteSelectionAsSubAction(
+          eNone,
+          aOptions.contains(InsertElementOption::SplitAncestorInlineElements)
+              ? eStrip
+              : eNoStrip);
      if (NS_FAILED(rv)) {
        NS_WARNING(
            "EditorBase::DeleteSelectionAsSubAction(eNone, eNoStrip) failed");
@ -2235,6 +2245,28 @@ nsresult HTMLEditor::InsertElementAtSelectionAsAction(
    return NS_ERROR_FAILURE;
  }

+  if (aOptions.contains(InsertElementOption::SplitAncestorInlineElements)) {
+    if (const RefPtr<Element> topmostInlineElement = Element::FromNodeOrNull(
+            HTMLEditUtils::GetMostDistantAncestorInlineElement(
+                *pointToInsert.ContainerAs<nsIContent>(),
+                BlockInlineCheck::UseComputedDisplayOutsideStyle,
+                editingHost))) {
+      Result<SplitNodeResult, nsresult> splitInlinesResult =
+          SplitNodeDeepWithTransaction(
+              *topmostInlineElement, pointToInsert,
+              SplitAtEdges::eDoNotCreateEmptyContainer);
+      if (MOZ_UNLIKELY(splitInlinesResult.isErr())) {
+        NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
+        return splitInlinesResult.unwrapErr();
+      }
+      splitInlinesResult.inspect().IgnoreCaretPointSuggestion();
+      auto splitPoint =
+          splitInlinesResult.inspect().AtSplitPoint<EditorDOMPoint>();
+      if (MOZ_LIKELY(splitPoint.IsSet())) {
+        pointToInsert = std::move(splitPoint);
+      }
+    }
+  }
  {
    Result<CreateElementResult, nsresult> insertElementResult =
        InsertNodeIntoProperAncestorWithTransaction<Element>(
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@ -266,9 +266,17 @@ class HTMLEditor final : public EditorBase,
  [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult
  InsertParagraphSeparatorAsAction(nsIPrincipal* aPrincipal = nullptr);

-  MOZ_CAN_RUN_SCRIPT nsresult
-  InsertElementAtSelectionAsAction(Element* aElement, bool aDeleteSelection,
-                                   nsIPrincipal* aPrincipal = nullptr);
+  enum class InsertElementOption {
+    // Delete selection if set, otherwise, insert aElement at start or end of
+    // selection.
+    DeleteSelection,
+    // Whether split all inline ancestors or not.
+    SplitAncestorInlineElements,
+  };
+  using InsertElementOptions = EnumSet<InsertElementOption>;
+  MOZ_CAN_RUN_SCRIPT nsresult InsertElementAtSelectionAsAction(
+      Element* aElement, const InsertElementOptions aOptions,
+      nsIPrincipal* aPrincipal = nullptr);

  MOZ_CAN_RUN_SCRIPT nsresult InsertLinkAroundSelectionAsAction(
      Element* aAnchorElement, nsIPrincipal* aPrincipal = nullptr);
--- a/editor/libeditor/HTMLEditorCommands.cpp
+++ b/editor/libeditor/HTMLEditorCommands.cpp
@ -1234,9 +1234,18 @@ nsresult InsertTagCommand::DoCommand(Command aCommand, EditorBase& aEditorBase,
  if (NS_WARN_IF(!newElement)) {
    return NS_ERROR_FAILURE;
  }
+  HTMLEditor::InsertElementOptions options{
+      HTMLEditor::InsertElementOption::DeleteSelection};
+  // We did insert <img> without splitting ancestor inline elements, but the
+  // other browsers split them.  Therefore, let's do it only when the document
+  // is content.
+  if (tagName == nsGkAtoms::img &&
+      htmlEditor->GetDocument()->IsContentDocument()) {
+    options += HTMLEditor::InsertElementOption::SplitAncestorInlineElements;
+  }
  nsresult rv =
      MOZ_KnownLive(htmlEditor)
-          ->InsertElementAtSelectionAsAction(newElement, true, aPrincipal);
+          ->InsertElementAtSelectionAsAction(newElement, options, aPrincipal);
  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                       "HTMLEditor::InsertElementAtSelectionAsAction() failed");
  return rv;
@ -1297,9 +1306,17 @@ nsresult InsertTagCommand::DoCommandParam(Command aCommand,
    return rv;
  }

+  HTMLEditor::InsertElementOptions options{
+      HTMLEditor::InsertElementOption::DeleteSelection};
+  // We did insert <img> without splitting ancestor inline elements, but the
+  // other browsers split them.  Therefore, let's do it only when the document
+  // is content.
+  if (htmlEditor->GetDocument()->IsContentDocument()) {
+    options += HTMLEditor::InsertElementOption::SplitAncestorInlineElements;
+  }
  nsresult rv =
      MOZ_KnownLive(htmlEditor)
-          ->InsertElementAtSelectionAsAction(newElement, true, aPrincipal);
+          ->InsertElementAtSelectionAsAction(newElement, options, aPrincipal);
  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                       "HTMLEditor::InsertElementAtSelectionAsAction() failed");
  return rv;
--- a/editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js
+++ b/editor/libeditor/tests/browserscope/lib/richtext2/currentStatus.js
@ -266,6 +266,9 @@ const knownFailures = {
    "I-Proposed-IIMG:._IMG-1_SO-dM": true,
    "I-Proposed-IIMG:._IMG-1_SO-body": true,
    "I-Proposed-IIMG:._IMG-1_SO-div": true,
+    "I-Proposed-IIMG:url_IMG-1_SO-dM": true,
+    "I-Proposed-IIMG:url_IMG-1_SO-body": true,
+    "I-Proposed-IIMG:url_IMG-1_SO-div": true,
    "Q-Proposed-CONTENTREADONLY_TEXT-1-dM": !SpecialPowers.getBoolPref("dom.document.edit_command.contentReadOnly.enabled", false),
    "Q-Proposed-CONTENTREADONLY_TEXT-1-body": !SpecialPowers.getBoolPref("dom.document.edit_command.contentReadOnly.enabled", false),
    "Q-Proposed-CONTENTREADONLY_TEXT-1-div": !SpecialPowers.getBoolPref("dom.document.edit_command.contentReadOnly.enabled", false),
@ -867,6 +870,9 @@ const knownFailures = {
    "I-Proposed-IIMG:._IMG-1_SO-dM": true,
    "I-Proposed-IIMG:._IMG-1_SO-body": true,
    "I-Proposed-IIMG:._IMG-1_SO-div": true,
+    "I-Proposed-IIMG:url_IMG-1_SO-dM": true,
+    "I-Proposed-IIMG:url_IMG-1_SO-body": true,
+    "I-Proposed-IIMG:url_IMG-1_SO-div": true,
    "I-Proposed-IHTML:BR_TEXT-1_SC-dM": true,
    "I-Proposed-IHTML:BR_TEXT-1_SC-body": true,
    "I-Proposed-IHTML:BR_TEXT-1_SC-div": true,
--- a/testing/web-platform/meta/editing/run/insertimage.html.ini
+++ b/testing/web-platform/meta/editing/run/insertimage.html.ini
@ -1,15 +1,6 @@
 [insertimage.html]
  expected:
    if (os == "android") and fission: [OK, TIMEOUT]
-  [[["insertimage","/img/lion.svg"\]\] "foo{<span style=color:#aBcDeF>bar</span>}baz" compare innerHTML]
-    expected: FAIL
-
-  [[["insertimage","/img/lion.svg"\]\] "foo{<b>bar</b>}baz" compare innerHTML]
-    expected: FAIL
-
-  [[["insertimage","/img/lion.svg"\]\] "foo{<span>bar</span>}baz" compare innerHTML]
-    expected: FAIL
-
  [[["stylewithcss","true"\],["defaultparagraphseparator","div"\],["insertimage","/img/lion.svg"\]\] "<p>foo[bar<p style=color:blue>baz\]quz" compare innerHTML]
    expected: FAIL

--- a/testing/web-platform/tests/editing/data/insertimage.js
+++ b/testing/web-platform/tests/editing/data/insertimage.js
@ -12,7 +12,7 @@ var browserTests = [
    {"insertimage":[false,false,"",false,false,""]}],
 ["<span>foo[</span><span>]bar</span>",
    [["insertimage","/img/lion.svg"]],
-    "<span>foo<img src=\"/img/lion.svg\">{}</span><span>bar</span>",
+    "<span>foo</span><img src=\"/img/lion.svg\"><span>bar</span>",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo[bar]baz",
@ -27,17 +27,17 @@ var browserTests = [
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo<span style=color:#aBcDeF>[bar]</span>baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<span style=\"color:rgb(171, 205, 239)\"><img src=\"/img/lion.svg\">{}</span>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo<span style=color:#aBcDeF>{bar}</span>baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<span style=\"color:rgb(171, 205, 239)\"><img src=\"/img/lion.svg\">{}</span>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo{<span style=color:#aBcDeF>bar</span>}baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<span style=\"color:rgb(171, 205, 239)\"><img src=\"/img/lion.svg\">{}</span>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["[foo<span style=color:#aBcDeF>bar]</span>baz",
@ -62,57 +62,57 @@ var browserTests = [
    {"stylewithcss":[false,true,"",false,false,""],"insertimage":[false,false,"",false,false,""]}],
 ["foo<span style=color:#aBcDeF>[bar</span>baz]",
    [["insertimage","/img/lion.svg"]],
-    "foo<span style=\"color:rgb(171, 205, 239)\"><img src=\"/img/lion.svg\">{}</span>",
+    "foo<img src=\"/img/lion.svg\">",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo<span style=color:#aBcDeF>{bar</span>baz}",
    [["insertimage","/img/lion.svg"]],
-    "foo<span style=\"color:rgb(171, 205, 239)\"><img src=\"/img/lion.svg\">{}</span>",
+    "foo<img src=\"/img/lion.svg\">",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo<span style=color:#aBcDeF>[bar</span><span style=color:#fEdCbA>baz]</span>quz",
    [["stylewithcss","true"],["insertimage","/img/lion.svg"]],
-    "foo<span style=\"color:rgb(171, 205, 239)\"><img src=\"/img/lion.svg\">{}</span>quz",
+    "foo<img src=\"/img/lion.svg\">quz",
    [true,true],
    {"stylewithcss":[false,false,"",false,true,""],"insertimage":[false,false,"",false,false,""]}],
 ["foo<span style=color:#aBcDeF>[bar</span><span style=color:#fEdCbA>baz]</span>quz",
    [["stylewithcss","false"],["insertimage","/img/lion.svg"]],
-    "foo<span style=\"color:rgb(171, 205, 239)\"><img src=\"/img/lion.svg\">{}</span>quz",
+    "foo<img src=\"/img/lion.svg\">quz",
    [true,true],
    {"stylewithcss":[false,true,"",false,false,""],"insertimage":[false,false,"",false,false,""]}],
 ["foo<b>[bar]</b>baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<b><img src=\"/img/lion.svg\">{}</b>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo<b>{bar}</b>baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<b><img src=\"/img/lion.svg\">{}</b>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo{<b>bar</b>}baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<b><img src=\"/img/lion.svg\">{}</b>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo<span>[bar]</span>baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<span><img src=\"/img/lion.svg\">{}</span>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo<span>{bar}</span>baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<span><img src=\"/img/lion.svg\">{}</span>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["foo{<span>bar</span>}baz",
    [["insertimage","/img/lion.svg"]],
-    "foo<span><img src=\"/img/lion.svg\">{}</span>baz",
+    "foo<img src=\"/img/lion.svg\">baz",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["<b>foo[bar</b><i>baz]quz</i>",
    [["insertimage","/img/lion.svg"]],
-    "<b>foo<img src=\"/img/lion.svg\">{}</b><i>quz</i>",
+    "<b>foo</b><img src=\"/img/lion.svg\"><i>quz</i>",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
 ["<p>foo</p><p>[bar]</p><p>baz</p>",
@ -355,4 +355,32 @@ var browserTests = [
    "foo<img src=\"/\u65E5\u672C\u8A9E\u30D1\u30B9/lion.svg\">{}bar",
    [true],
    {"insertimage":[false,false,"",false,false,""]}],
+["<div>{}<br></div>",
+    [["insertimage","/img/lion.svg"]],
+    ["<div><img src=\"/img/lion.svg\"></div>",
+     "<div><img src=\"/img/lion.svg\"><br></div>"],
+    [true],
+    {}],
+["<div><b>{}<br></b></div>",
+    [["insertimage","/img/lion.svg"]],
+    ["<div><img src=\"/img/lion.svg\"></div>",
+     "<div><img src=\"/img/lion.svg\"><b><br></b></div>"],
+    [true],
+    {}],
+["<div><span>{}<br></span></div>",
+    [["insertimage","/img/lion.svg"]],
+    ["<div><img src=\"/img/lion.svg\"></div>",
+     "<div><img src=\"/img/lion.svg\"><span><br></span></div>"],
+    [true],
+    {}],
+["<div><b>A[]B</b></div>",
+    [["insertimage","/img/lion.svg"]],
+    "<div><b>A</b><img src=\"/img/lion.svg\"><b>B</b></div>",
+    [true],
+    {}],
+["<div><span>A[]B</span></div>",
+    [["insertimage","/img/lion.svg"]],
+    "<div><span>A</span><img src=\"/img/lion.svg\"><span>B</span></div>",
+    [true],
+    {}],
 ]