Bug 1415509 - part 3: WSRunObject::InsertText() should take |const EditorRawDOMPoint&| as input argument and |EditorRawDOMPoint*| as out argument instead of a set of container, child and offset of the child in the container as in/out argument r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 09 Nov 2017 13:24:06 +0900
changeset 696171 818963f2e67f4f469f6db9d05b3e8ad8d43657d7
parent 696170 6693ba51906afd16d130e694752b04cb7bed6dbd
child 739809 be3a36776dd26e8ccbee3fe57ff9531237fa7e60
push id88660
push usermasayuki@d-toybox.com
push dateFri, 10 Nov 2017 09:13:09 +0000
reviewersm_kato
bugs1415509
milestone58.0a1
Bug 1415509 - part 3: WSRunObject::InsertText() should take |const EditorRawDOMPoint&| as input argument and |EditorRawDOMPoint*| as out argument instead of a set of container, child and offset of the child in the container as in/out argument r?m_kato Like EditorBase::InsertTextImpl(), WSRunObject::InsertText() is really messy. So, it should take same arguments as EditorBase::InsertTextImpl(). MozReview-Commit-ID: 5uKGaxKheRv
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/WSRunObject.cpp
editor/libeditor/WSRunObject.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -1361,21 +1361,17 @@ HTMLEditRules::WillInsertText(EditAction
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       return NS_OK;
     }
 
     WSRunObject wsObj(mHTMLEditor,
                       pointToInsert.Container(), pointToInsert.Offset());
-    nsCOMPtr<nsINode> selNode = pointToInsert.Container();
-    nsCOMPtr<nsIContent> selChild = pointToInsert.GetChildAtOffset();
-    int32_t selOffset = pointToInsert.Offset();
-    rv = wsObj.InsertText(*inString, address_of(selNode),
-                          address_of(selChild), &selOffset, doc);
+    rv = wsObj.InsertText(*doc, *inString, pointToInsert.AsRaw());
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     return NS_OK;
   }
 
   // aAction == kInsertText
 
@@ -1481,26 +1477,25 @@ HTMLEditRules::WillInsertText(EditAction
 
         nsDependentSubstring subStr(tString, oldPos, subStrLen);
         NS_ENSURE_STATE(mHTMLEditor);
         WSRunObject wsObj(mHTMLEditor, currentPoint.Container(),
                           currentPoint.Offset());
 
         // is it a tab?
         if (subStr.Equals(tabStr)) {
-          nsCOMPtr<nsINode> curNode = currentPoint.Container();
-          nsCOMPtr<nsIContent> selChild = currentPoint.GetChildAtOffset();
-          int32_t curOffset = currentPoint.Offset();
-          rv =
-            wsObj.InsertText(spacesStr, address_of(curNode),
-                             address_of(selChild), &curOffset, doc);
-          NS_ENSURE_SUCCESS(rv, rv);
+          EditorRawDOMPoint pointAfterInsertedSpaces;
+          rv = wsObj.InsertText(*doc, spacesStr, currentPoint.AsRaw(),
+                                &pointAfterInsertedSpaces);
+          if (NS_WARN_IF(NS_FAILED(rv))) {
+            return rv;
+          }
           pos++;
-          currentPoint.Set(curNode, curOffset);
-          pointToInsert = currentPoint;
+          currentPoint = pointAfterInsertedSpaces;
+          pointToInsert = pointAfterInsertedSpaces;
         }
         // is it a return?
         else if (subStr.Equals(newlineStr)) {
           nsCOMPtr<nsINode> curNode = currentPoint.Container();
           int32_t curOffset = currentPoint.Offset();
           nsCOMPtr<Element> br = wsObj.InsertBreak(address_of(curNode),
                                                    &curOffset,
                                                    nsIEditor::eNone);
@@ -1509,24 +1504,24 @@ HTMLEditRules::WillInsertText(EditAction
           if (br->GetNextSibling()) {
             pointToInsert.Set(br->GetNextSibling());
           } else {
             pointToInsert.Set(curNode, curNode->Length());
           }
           currentPoint.Set(curNode, curOffset);
           MOZ_ASSERT(currentPoint == pointToInsert);
         } else {
-          nsCOMPtr<nsINode> curNode = currentPoint.Container();
-          nsCOMPtr<nsIContent> selChild = currentPoint.GetChildAtOffset();
-          int32_t curOffset = currentPoint.Offset();
-          rv = wsObj.InsertText(subStr, address_of(curNode),
-                                address_of(selChild), &curOffset, doc);
-          NS_ENSURE_SUCCESS(rv, rv);
-          currentPoint.Set(curNode, curOffset);
-          pointToInsert = currentPoint;
+          EditorRawDOMPoint pointAfterInsertedString;
+          rv = wsObj.InsertText(*doc, subStr, currentPoint.AsRaw(),
+                                &pointAfterInsertedString);
+          if (NS_WARN_IF(NS_FAILED(rv))) {
+            return rv;
+          }
+          currentPoint = pointAfterInsertedString;
+          pointToInsert = pointAfterInsertedString;
         }
       }
     }
 
     // After this block, pointToInsert is updated by AutoTrackDOMPoint.
   }
 
   aSelection->SetInterlinePosition(false);
--- a/editor/libeditor/WSRunObject.cpp
+++ b/editor/libeditor/WSRunObject.cpp
@@ -228,113 +228,107 @@ WSRunObject::InsertBreak(nsCOMPtr<nsINod
     }
   }
 
   // ready, aim, fire!
   return mHTMLEditor->CreateBRImpl(aInOutParent, aInOutOffset, aSelect);
 }
 
 nsresult
-WSRunObject::InsertText(const nsAString& aStringToInsert,
-                        nsCOMPtr<nsINode>* aInOutParent,
-                        nsCOMPtr<nsIContent>* aInOutChildAtOffset,
-                        int32_t* aInOutOffset,
-                        nsIDocument* aDoc)
+WSRunObject::InsertText(nsIDocument& aDocument,
+                        const nsAString& aStringToInsert,
+                        const EditorRawDOMPoint& aPointToInsert,
+                        EditorRawDOMPoint* aPointAfterInsertedString)
 {
   // MOOSE: for now, we always assume non-PRE formatting.  Fix this later.
   // meanwhile, the pre case is handled in WillInsertText in
   // HTMLEditRules.cpp
 
   // MOOSE: for now, just getting the ws logic straight.  This implementation
   // is very slow.  Will need to replace edit rules impl with a more efficient
   // text sink here that does the minimal amount of searching/replacing/copying
 
-  NS_ENSURE_TRUE(aInOutParent && aInOutOffset && aDoc, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!aPointToInsert.IsSet())) {
+    return NS_ERROR_INVALID_ARG;
+  }
+  MOZ_ASSERT(aPointToInsert.IsSet());
+
 
   if (aStringToInsert.IsEmpty()) {
+    if (aPointAfterInsertedString) {
+      *aPointAfterInsertedString = aPointToInsert;
+    }
     return NS_OK;
   }
 
+  EditorDOMPoint pointToInsert(aPointToInsert);
   nsAutoString theString(aStringToInsert);
 
   WSFragment *beforeRun, *afterRun;
-  FindRun(*aInOutParent, *aInOutOffset, &beforeRun, false);
-  FindRun(*aInOutParent, *aInOutOffset, &afterRun, true);
+  FindRun(pointToInsert.Container(), pointToInsert.Offset(),
+          &beforeRun, false);
+  FindRun(pointToInsert.Container(), pointToInsert.Offset(),
+          &afterRun, true);
 
   {
     // Some scoping for AutoTrackDOMPoint.  This will track our insertion
     // point while we tweak any surrounding whitespace
-    AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater, aInOutParent,
-                              aInOutOffset);
+    AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater, &pointToInsert);
 
-    bool maybeModified = false;
     // Handle any changes needed to ws run after inserted text
     if (!afterRun || afterRun->mType & WSType::trailingWS) {
       // Don't need to do anything.  Just insert text.  ws won't change.
     } else if (afterRun->mType & WSType::leadingWS) {
       // Delete the leading ws that is after insertion point, because it
       // would become significant after text inserted.
       nsresult rv =
-        DeleteChars(*aInOutParent, *aInOutOffset, afterRun->mEndNode,
-                    afterRun->mEndOffset);
+        DeleteChars(pointToInsert.Container(), pointToInsert.Offset(),
+                    afterRun->mEndNode, afterRun->mEndOffset);
       NS_ENSURE_SUCCESS(rv, rv);
-      maybeModified = true;
     } else if (afterRun->mType == WSType::normalWS) {
       // Try to change an nbsp to a space, if possible, just to prevent nbsp
       // proliferation
-      nsresult rv = CheckLeadingNBSP(afterRun, *aInOutParent, *aInOutOffset);
+      nsresult rv = CheckLeadingNBSP(afterRun, pointToInsert.Container(),
+                                     pointToInsert.Offset());
       NS_ENSURE_SUCCESS(rv, rv);
-      maybeModified = true;
     }
 
     // Handle any changes needed to ws run before inserted text
     if (!beforeRun || beforeRun->mType & WSType::leadingWS) {
       // Don't need to do anything.  Just insert text.  ws won't change.
     } else if (beforeRun->mType & WSType::trailingWS) {
       // Need to delete the trailing ws that is before insertion point, because
       // it would become significant after text inserted.
       nsresult rv =
         DeleteChars(beforeRun->mStartNode, beforeRun->mStartOffset,
-                    *aInOutParent, *aInOutOffset);
+                    pointToInsert.Container(), pointToInsert.Offset());
       NS_ENSURE_SUCCESS(rv, rv);
-      maybeModified = true;
     } else if (beforeRun->mType == WSType::normalWS) {
       // Try to change an nbsp to a space, if possible, just to prevent nbsp
       // proliferation
-      nsresult rv = CheckTrailingNBSP(beforeRun, *aInOutParent, *aInOutOffset);
+      nsresult rv = CheckTrailingNBSP(beforeRun, pointToInsert.Container(),
+                                      pointToInsert.Offset());
       NS_ENSURE_SUCCESS(rv, rv);
-      maybeModified = true;
     }
 
-    // The child node may be changed.  So, even though getting child at offset
-    // is expensive, we need to do it here.
-    if (maybeModified) {
-      if ((*aInOutParent)->HasChildren()) {
-        if (*aInOutOffset == 0) {
-          *aInOutChildAtOffset = (*aInOutParent)->GetFirstChild();
-        } else {
-          *aInOutChildAtOffset = (*aInOutParent)->GetChildAt(*aInOutOffset);
-        }
-      } else {
-        *aInOutChildAtOffset = nullptr;
-      }
-    }
+    // After this block, pointToInsert is modified by AutoTrackDOMPoint.
   }
 
   // Next up, tweak head and tail of string as needed.  First the head: there
   // are a variety of circumstances that would require us to convert a leading
   // ws char into an nbsp:
 
   if (nsCRT::IsAsciiSpace(theString[0])) {
     // We have a leading space
     if (beforeRun) {
       if (beforeRun->mType & WSType::leadingWS) {
         theString.SetCharAt(nbsp, 0);
       } else if (beforeRun->mType & WSType::normalWS) {
-        WSPoint wspoint = GetCharBefore(*aInOutParent, *aInOutOffset);
+        WSPoint wspoint =
+          GetCharBefore(pointToInsert.Container(), pointToInsert.Offset());
         if (wspoint.mTextNode && nsCRT::IsAsciiSpace(wspoint.mChar)) {
           theString.SetCharAt(nbsp, 0);
         }
       }
     } else if (mStartReason & WSType::block || mStartReason == WSType::br) {
       theString.SetCharAt(nbsp, 0);
     }
   }
@@ -343,17 +337,18 @@ WSRunObject::InsertText(const nsAString&
   uint32_t lastCharIndex = theString.Length() - 1;
 
   if (nsCRT::IsAsciiSpace(theString[lastCharIndex])) {
     // We have a leading space
     if (afterRun) {
       if (afterRun->mType & WSType::trailingWS) {
         theString.SetCharAt(nbsp, lastCharIndex);
       } else if (afterRun->mType & WSType::normalWS) {
-        WSPoint wspoint = GetCharAfter(*aInOutParent, *aInOutOffset);
+        WSPoint wspoint =
+          GetCharAfter(pointToInsert.Container(), pointToInsert.Offset());
         if (wspoint.mTextNode && nsCRT::IsAsciiSpace(wspoint.mChar)) {
           theString.SetCharAt(nbsp, lastCharIndex);
         }
       }
     } else if (mEndReason & WSType::block) {
       theString.SetCharAt(nbsp, lastCharIndex);
     }
   }
@@ -372,27 +367,22 @@ WSRunObject::InsertText(const nsAString&
         prevWS = true;
       }
     } else {
       prevWS = false;
     }
   }
 
   // Ready, aim, fire!
-  EditorRawDOMPoint pointToInsert(*aInOutParent, *aInOutChildAtOffset,
-                                  *aInOutOffset);
-  EditorRawDOMPoint pointAfterInsertedString;
-  nsresult rv = mHTMLEditor->InsertTextImpl(*aDoc, theString, pointToInsert,
-                                            &pointAfterInsertedString);
+  nsresult rv =
+    mHTMLEditor->InsertTextImpl(aDocument, theString, pointToInsert.AsRaw(),
+                                aPointAfterInsertedString);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return NS_OK;
   }
-  *aInOutParent = pointAfterInsertedString.Container();
-  *aInOutChildAtOffset = pointAfterInsertedString.GetChildAtOffset();
-  *aInOutOffset = pointAfterInsertedString.Offset();
   return NS_OK;
 }
 
 nsresult
 WSRunObject::DeleteWSBackward()
 {
   WSPoint point = GetCharBefore(mNode, mOffset);
   NS_ENSURE_TRUE(point.mTextNode, NS_OK);  // nothing to delete
--- a/editor/libeditor/WSRunObject.h
+++ b/editor/libeditor/WSRunObject.h
@@ -214,24 +214,41 @@ public:
   // InsertBreak inserts a br node at {aInOutParent,aInOutOffset}
   // and makes any needed adjustments to ws around that point.
   // example of fixup: normalws after {aInOutParent,aInOutOffset}
   //                   needs to begin with nbsp.
   already_AddRefed<dom::Element> InsertBreak(nsCOMPtr<nsINode>* aInOutParent,
                                              int32_t* aInOutOffset,
                                              nsIEditor::EDirection aSelect);
 
-  // InsertText inserts a string at {aInOutParent,aInOutOffset} and makes any
-  // needed adjustments to ws around that point.  Example of fixup:
-  // trailingws before {aInOutParent,aInOutOffset} needs to be removed.
-  nsresult InsertText(const nsAString& aStringToInsert,
-                      nsCOMPtr<nsINode>* aInOutNode,
-                      nsCOMPtr<nsIContent>* aInOutChildAtOffset,
-                      int32_t* aInOutOffset,
-                      nsIDocument* aDoc);
+  /**
+   * InsertTextImpl() inserts aStringToInsert to aPointToInsert and makes any
+   * needed adjustments to white spaces around that point. E.g., trailing white
+   * spaces before aPointToInsert needs to be removed.
+   * This calls EditorBase::InsertTextImpl() after adjusting white spaces.
+   * So, please refer the method's explanation to know what this method exactly
+   * does.
+   *
+   * @param aDocument       The document of this editor.
+   * @param aStringToInsert The string to insert.
+   * @param aPointToInser   The point to insert aStringToInsert.
+   *                        Must be valid DOM point.
+   * @param aPointAfterInsertedString
+   *                        The point after inserted aStringToInsert.
+   *                        So, when this method actually inserts string,
+   *                        this is set to a point in the text node.
+   *                        Otherwise, this may be set to aPointToInsert.
+   * @return                When this succeeds to insert the string or
+   *                        does nothing during composition, returns NS_OK.
+   *                        Otherwise, an error code.
+   */
+  nsresult InsertText(nsIDocument& aDocument,
+                      const nsAString& aStringToInsert,
+                      const EditorRawDOMPoint& aPointToInsert,
+                      EditorRawDOMPoint* aPointAfterInsertedString = nullptr);
 
   // DeleteWSBackward deletes a single visible piece of ws before the ws
   // point (the point to create the wsRunObject, passed to its constructor).
   // It makes any needed conversion to adjacent ws to retain its
   // significance.
   nsresult DeleteWSBackward();
 
   // DeleteWSForward deletes a single visible piece of ws after the ws point