From fb1d603cfd6c85b66411c2172fe464f57afc9598 Mon Sep 17 00:00:00 2001
From: Masayuki Nakano <masayuki@d-toybox.com>
Date: Mon, 21 Apr 2025 23:49:31 +0000
Subject: [PATCH] Bug 1961521 - Make `HTMLEditor::HandleInsertText()` collapse
 `Selection` properly after deleting composition r=m_kato

When inserting text or deleting text is for updating composition string,
`InsertTextWithTransaction()` does not suggest caret position because in the
most cases, `CompositionTransaction` updating `Selection` properly.  However,
if it's deleting existing composition (i.e., replacing the composition with
empty string), `HTMLEditor::HandleInsertText()` doesn't use
`CompositionTransaction` to update composition.  Instead, it uses the suggested
caret position from `InsertTextWithTransaction()` even though it's always unset.
Therefore, if the composition string follows some collapsible white-spaces and
they are replaced, `Selection` is automatically moved to the start position of
the replaced range and `HTMLEditor::HandleInsertText()` does not maintain the
caret position.

Therefore, this patch make it collapse `Selection` to end of the inserted text
where the composition string was.

Differential Revision: https://phabricator.services.mozilla.com/D246087
---
 editor/libeditor/HTMLEditSubActionHandler.cpp | 25 ++++++++-------
 ...window_composition_text_querycontent.xhtml | 32 +++++++++++++++++++
 2 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp
index 95790fe06792..f55e082449c5 100644
--- a/editor/libeditor/HTMLEditSubActionHandler.cpp
+++ b/editor/libeditor/HTMLEditSubActionHandler.cpp
@@ -1199,6 +1199,10 @@ Result<EditActionResult, nsresult> HTMLEditor::HandleInsertText(
       }
       InsertTextResult insertEmptyTextResult =
           insertEmptyTextResultOrError.unwrap();
+      // InsertTextWithTransaction() doesn not suggest caret position if it's
+      // called for IME composition. However, for the safety, let's ignore the
+      // caret position explicitly.
+      insertEmptyTextResult.IgnoreCaretPointSuggestion();
       nsresult rv = EnsureNoFollowingUnnecessaryLineBreak(
           insertEmptyTextResult.EndOfInsertedTextRef());
       if (NS_FAILED(rv)) {
@@ -1236,21 +1240,20 @@ Result<EditActionResult, nsresult> HTMLEditor::HandleInsertText(
       if (MOZ_UNLIKELY(insertPaddingBRElementResultOrError.isErr())) {
         NS_WARNING(
             "HTMLEditor::InsertPaddingBRElementIfNeeded(eNoStrip) failed");
-        insertEmptyTextResult.IgnoreCaretPointSuggestion();
         return insertPaddingBRElementResultOrError.propagateErr();
       }
       insertPaddingBRElementResultOrError.unwrap().IgnoreCaretPointSuggestion();
-      rv = insertEmptyTextResult.SuggestCaretPointTo(
-          *this, {SuggestCaret::OnlyIfHasSuggestion,
-                  SuggestCaret::OnlyIfTransactionsAllowedToDoIt,
-                  SuggestCaret::AndIgnoreTrivialError});
-      if (NS_FAILED(rv)) {
-        NS_WARNING("CaretPoint::SuggestCaretPointTo() failed");
-        return Err(rv);
+      // Then, collapse caret after the empty text inserted position, i.e.,
+      // whether the removed composition string was.
+      if (AllowsTransactionsToChangeSelection()) {
+        nsresult rv = CollapseSelectionTo(endOfInsertedText);
+        if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
+          return Err(rv);
+        }
+        NS_WARNING_ASSERTION(
+            rv != NS_SUCCESS_EDITOR_BUT_IGNORED_TRIVIAL_ERROR,
+            "CaretPoint::SuggestCaretPointTo() failed, but ignored");
       }
-      NS_WARNING_ASSERTION(
-          rv != NS_SUCCESS_EDITOR_BUT_IGNORED_TRIVIAL_ERROR,
-          "CaretPoint::SuggestCaretPointTo() failed, but ignored");
       return EditActionResult::HandledResult();
     }
 
diff --git a/widget/tests/window_composition_text_querycontent.xhtml b/widget/tests/window_composition_text_querycontent.xhtml
index 84507a4267e9..fd31bb561697 100644
--- a/widget/tests/window_composition_text_querycontent.xhtml
+++ b/widget/tests/window_composition_text_querycontent.xhtml
@@ -11404,6 +11404,37 @@ function runPaddingLineBreakTest() {
   contenteditable.innerHTML = "";
 }
 
+function runCancelCompositionAfterCollapseWhileSpace() {
+  const selection = windowOfContenteditable.getSelection();
+  contenteditable.innerHTML = "<p>A&nbsp;</p>";
+  contenteditable.focus();
+  selection.collapse(
+    contenteditable.querySelector("[contenteditable] > p").firstChild,
+    "A ".length
+  );
+  const description = "runCancelCompositionAfterCollapseWhileSpace";
+  synthesizeCompositionChange({
+    composition: {string: "B", clauses: [{length: 1, attr: COMPOSITION_ATTR_RAW_CLAUSE}]},
+  });
+  is(
+    contenteditable.innerHTML.replaceAll("&nbsp;", " "),
+    "<p>A B</p>",
+    `${description}: Composing string "B" should be inserted after the collapsible white-space`
+  );
+  synthesizeComposition({type: "compositioncommit", data: ""});
+  is(
+    contenteditable.innerHTML,
+    kNewWhiteSpaceNormalizerEnabled ? "<p>A&nbsp;</p>" : "<p>A <br></p>",
+    `${description}: Canceling the composition should keep the preceding collapsible white-space visible`
+  );
+  synthesizeKey("B");
+  is(
+    contenteditable.innerHTML,
+    "<p>A B</p>",
+    `${description}: Typing "B" after canceling composition should cause inserting it after the collpsible white-space`
+  );
+}
+
 async function runTest()
 {
   await SpecialPowers.pushPrefEnv({
@@ -11455,6 +11486,7 @@ async function runTest()
   runBug1571375Test();
   runBug1675313Test();
   runCommitCompositionWithSpaceKey();
+  runCancelCompositionAfterCollapseWhileSpace();
   runCompositionWithSelectionChange();
   runCompositionWithClick();
   runForceCommitTest();