Bug 1649980 - part 7: Redesign WSRunObject::MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace() r=m_kato

Same as the previous patch, it can be split to computation part and
modifying the DOM tree part.  Then, the former can be in `TextFragmentData`
and the latter can be done by the caller which is only
`WSRunObject::InsertText()`.

Depends on D82699

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

dom/events/test/test_text_event_in_content.html

@@ -12,6 +12,7 @@
 <div contenteditable id="editor"><p><br></p></div>
 <script>
 SimpleTest.waitForExplicitFinish();
+SimpleTest.expectAssertions(2);  // Assertions in WSRunScanner::TextFragmentData::GetInclusiveNextNBSPPointIfNeedToReplaceWithASCIIWhiteSpace()
 SimpleTest.waitForFocus(async function doTests() {
   await SpecialPowers.pushPrefEnv({set: [
     ["dom.compositionevent.text.dispatch_only_system_group_in_content", true],

editor/libeditor/WSRunObject.cpp

@@ -415,13 +415,22 @@ nsresult WSRunObject::InsertText(Document& aDocument,
                  PointPosition::StartOfFragment ||
              pointPositionWithVisibleWhiteSpacesAtEnd ==
                  PointPosition::MiddleOfFragment) {
-      nsresult rv = MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace(
+      EditorDOMPointInText atNBSPReplacedWithASCIIWhiteSpace =
-          visibleWhiteSpacesAtEnd.ref(), pointToInsert);
+          textFragmentDataAtEnd
-      if (NS_FAILED(rv)) {
+              .GetInclusiveNextNBSPPointIfNeedToReplaceWithASCIIWhiteSpace(
-        NS_WARNING(
+                  pointToInsert);
-            "WSRunObject::MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace() "
+      if (atNBSPReplacedWithASCIIWhiteSpace.IsSet()) {
-            "failed");
+        AutoTransactionsConserveSelection dontChangeMySelection(mHTMLEditor);
-        return rv;
+        nsresult rv =
+            MOZ_KnownLive(mHTMLEditor)
+                .ReplaceTextWithTransaction(
+                    MOZ_KnownLive(
+                        *atNBSPReplacedWithASCIIWhiteSpace.ContainerAsText()),
+                    atNBSPReplacedWithASCIIWhiteSpace.Offset(), 1, u" "_ns);
+        if (NS_FAILED(rv)) {
+          NS_WARNING("HTMLEditor::ReplaceTextWithTransaction() failed");
+          return rv;
+        }
       }
     }
 
@@ -2154,19 +2163,28 @@ EditorDOMPointInText WSRunScanner::TextFragmentData::
   return atPreviousChar;
 }
 
-nsresult WSRunObject::MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace(
+EditorDOMPointInText WSRunScanner::TextFragmentData::
-    const VisibleWhiteSpacesData& aVisibleWhiteSpacesData,
+    GetInclusiveNextNBSPPointIfNeedToReplaceWithASCIIWhiteSpace(
-    const EditorDOMPoint& aPoint) {
+        const EditorDOMPoint& aPointToInsert) const {
-  MOZ_ASSERT(aPoint.IsSetAndValid());
+  MOZ_ASSERT(aPointToInsert.IsSetAndValid());
+  MOZ_ASSERT(CreateVisibleWhiteSpacesData().isSome());
+  NS_ASSERTION(
+      CreateVisibleWhiteSpacesData().ref().ComparePoint(aPointToInsert) ==
+              PointPosition::StartOfFragment ||
+          CreateVisibleWhiteSpacesData().ref().ComparePoint(aPointToInsert) ==
+              PointPosition::MiddleOfFragment,
+      "Inclusive next char of aPointToInsert should be in the visible "
+      "white-spaces");
 
   // Try to change an nbsp to a space, if possible, just to prevent nbsp
   // proliferation This routine is called when we are about to make this point
   // in the ws abut an inserted text, so we don't have to worry about what is
   // before it.  What is before it now will end up before the inserted text.
-  EditorDOMPointInText atNextChar = GetInclusiveNextEditableCharPoint(aPoint);
+  EditorDOMPointInText atNextChar =
+      GetInclusiveNextEditableCharPoint(aPointToInsert);
   if (!atNextChar.IsSet() || NS_WARN_IF(atNextChar.IsEndOfContainer()) ||
       !atNextChar.IsCharNBSP()) {
-    return NS_OK;
+    return EditorDOMPointInText();
   }
 
   EditorDOMPointInText atNextCharOfNextCharOfNBSP =
@@ -2176,25 +2194,22 @@ nsresult WSRunObject::MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace(
     // need to replace it with same character.
     if (!atNextCharOfNextCharOfNBSP.IsEndOfContainer() &&
         atNextCharOfNextCharOfNBSP.IsCharASCIISpace()) {
-      return NS_OK;
+      return EditorDOMPointInText();
     }
-  }
+    return atNextChar;
-  // If the NBSP is last character in the hard line, we don't need to
-  // replace it because it's required to render multiple white-spaces.
-  else if (!aVisibleWhiteSpacesData.EndsByNormalText() &&
-           !aVisibleWhiteSpacesData.EndsBySpecialContent() &&
-           !aVisibleWhiteSpacesData.EndsByBRElement()) {
-    return NS_OK;
   }
 
-  AutoTransactionsConserveSelection dontChangeMySelection(mHTMLEditor);
+  // If the NBSP is last character in the hard line, we don't need to
-  nsresult rv = MOZ_KnownLive(mHTMLEditor)
+  // replace it because it's required to render multiple white-spaces.
-                    .ReplaceTextWithTransaction(
+  Maybe<VisibleWhiteSpacesData> visibleWhiteSpaces =
-                        MOZ_KnownLive(*atNextChar.ContainerAsText()),
+      CreateVisibleWhiteSpacesData();
-                        atNextChar.Offset(), 1, u" "_ns);
+  if (!visibleWhiteSpaces.ref().EndsByNormalText() &&
-  NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+      !visibleWhiteSpaces.ref().EndsBySpecialContent() &&
-                       "HTMLEditor::ReplaceTextWithTransaction() failed");
+      !visibleWhiteSpaces.ref().EndsByBRElement()) {
-  return rv;
+    return EditorDOMPointInText();
+  }
+
+  return atNextChar;
 }
 
 nsresult WSRunObject::DeleteInvisibleASCIIWhiteSpacesInternal() {

editor/libeditor/WSRunObject.h

@@ -1023,6 +1023,19 @@ class MOZ_STACK_CLASS WSRunScanner {
     EditorDOMPointInText GetPreviousNBSPPointIfNeedToReplaceWithASCIIWhiteSpace(
         const EditorDOMPoint& aPointToInsert) const;
 
+    /**
+     * GetInclusiveNextNBSPPointIfNeedToReplaceWithASCIIWhiteSpace() may return
+     * an NBSP point which should be replaced with an ASCII white-space when
+     * the caller inserts text into aPointToInsert.
+     * Note that this is a helper method for the traditional white-space
+     * normalizer.  Don't use this with the new white-space normalizer.
+     * Must be called only when CreateVisibleWhiteSpacesData() returns some,
+     * and inclusive next char of aPointToInsert is in the range.
+     */
+    EditorDOMPointInText
+    GetInclusiveNextNBSPPointIfNeedToReplaceWithASCIIWhiteSpace(
+        const EditorDOMPoint& aPointToInsert) const;
+
     /**
      * CreateVisibleWhiteSpacesData() creates VisibleWhiteSpacesData.
      * That is zero or more white-spaces which are visible.
@@ -1250,20 +1263,6 @@ class MOZ_STACK_CLASS WSRunObject final : public WSRunScanner {
   [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult NormalizeWhiteSpacesAtEndOf(
       const VisibleWhiteSpacesData& aVisibleWhiteSpacesData);
 
-  /**
-   * MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace() replaces an NBSP at
-   * the point with an ASCII white-space only when the point is an NBSP and
-   * it's replaceable with an ASCII white-space.
-   *
-   * @param aPoint      If point in a text node, the character is checked
-   *                    whether it's an NBSP or not.  Otherwise, first
-   *                    character of next editable text node is checked.
-   */
-  [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult
-  MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace(
-      const VisibleWhiteSpacesData& aVisibleWhiteSpacesData,
-      const EditorDOMPoint& aPoint);
-
   /**
    * See explanation of the static method for this.
    */

editor/libeditor/tests/test_contenteditable_text_input_handling.html

@@ -20,6 +20,7 @@
 <script class="testbody" type="application/javascript">
 
 SimpleTest.waitForExplicitFinish();
+SimpleTest.expectAssertions(1);  // Assertions in WSRunScanner::TextFragmentData::GetInclusiveNextNBSPPointIfNeedToReplaceWithASCIIWhiteSpace()
 SimpleTest.waitForFocus(runTests);
 
 const kLF = !navigator.platform.indexOf("Win") ? "\r\n" : "\n";

widget/tests/test_composition_text_querycontent.xhtml

@@ -18,12 +18,14 @@
 <script class="testbody" type="application/javascript">
 <![CDATA[
 
-// If setting selection with eSetSelection event whose range is larger than
+// 3 assertions are: If setting selection with eSetSelection event whose range
-// the actual range, hits "Can only call this on frames that have been reflowed:
+// is larger than the actual range, hits "Can only call this on frames that have
+// been reflowed:
 // '!(GetStateBits() & NS_FRAME_FIRST_REFLOW) || (GetParent()->GetStateBits() &
 // NS_FRAME_TOO_DEEP_IN_FRAME_TREE)'" in nsTextFrame.cpp.
 // Strangely, this doesn't occur with RDP on Windows.
-SimpleTest.expectAssertions(0, 3);
+// 12 assertions are: assertions in WSRunScanner::TextFragmentData::GetInclusiveNextNBSPPointIfNeedToReplaceWithASCIIWhiteSpace()
+SimpleTest.expectAssertions(0, 3 + 12);
 SimpleTest.waitForExplicitFinish();
 window.openDialog("window_composition_text_querycontent.xhtml", "_blank", 
                   "chrome,width=600,height=600,noopener", window);