Bug 1423835 - part 1: Add EditorDOMPointBase::SetToEndOf() to initialize the instance at end of container node r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 07 Dec 2017 17:27:20 +0900
changeset 710432 5a3771318d15b0bd6c8186156e7a4833459e761f
parent 710431 b21a83083ca97d28411faf31d5f930a1632bb818
child 710433 024ac127d0043107bb964833f17825849eaef4dd
push id92824
push usermasayuki@d-toybox.com
push dateSun, 10 Dec 2017 04:04:12 +0000
reviewersm_kato
bugs1423835
milestone59.0a1
Bug 1423835 - part 1: Add EditorDOMPointBase::SetToEndOf() to initialize the instance at end of container node r?m_kato Editor code sometimes sets a DOM point to end of a node. In this case, we need to write |Set(node, node->Length())|. So, it should have |void SetToEndOf(const nsINode* aContainer)| for making meaning of the code clearer. MozReview-Commit-ID: 91shMCD2d84
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorDOMPoint.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditorDataTransfer.cpp
editor/libeditor/InsertNodeTransaction.cpp
editor/libeditor/SplitNodeTransaction.cpp
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2704,17 +2704,17 @@ EditorBase::InsertTextImpl(nsIDocument& 
   EditorRawDOMPoint pointToInsert = FindBetterInsertionPoint(aPointToInsert);
 
   // If a neighboring text node already exists, use that
   if (!pointToInsert.Container()->IsNodeOfType(nsINode::eTEXT)) {
     nsIContent* child = nullptr;
     if (!pointToInsert.IsStartOfContainer() &&
         (child = pointToInsert.GetPreviousSiblingOfChildAtOffset()) &&
         child->IsNodeOfType(nsINode::eTEXT)) {
-      pointToInsert.Set(child, child->Length());
+      pointToInsert.SetToEndOf(child);
     } else if (!pointToInsert.IsEndOfContainer() &&
                (child = pointToInsert.GetChildAtOffset()) &&
                child->IsNodeOfType(nsINode::eTEXT)) {
       pointToInsert.Set(child, 0);
     }
   }
 
   if (ShouldHandleIMEComposition()) {
--- a/editor/libeditor/EditorDOMPoint.h
+++ b/editor/libeditor/EditorDOMPoint.h
@@ -292,16 +292,30 @@ public:
     }
     mParent = aChild->GetParentNode();
     mChild = const_cast<nsIContent*>(aChild->AsContent());
     mOffset.reset();
     mIsChildInitialized = true;
   }
 
   /**
+   * SetToEndOf() sets this to the end of aContainer.  Then, mChild is always
+   * nullptr but marked as initialized and mOffset is always set.
+   */
+  void
+  SetToEndOf(const nsINode* aContainer)
+  {
+    MOZ_ASSERT(aContainer);
+    mParent = const_cast<nsINode*>(aContainer);
+    mChild = nullptr;
+    mOffset = mozilla::Some(mParent->Length());
+    mIsChildInitialized = true;
+  }
+
+  /**
    * Clear() makes the instance not point anywhere.
    */
   void
   Clear()
   {
     mParent = nullptr;
     mChild = nullptr;
     mOffset.reset();
@@ -703,17 +717,17 @@ public:
    */
   void InvalidateOffset()
   {
     if (mChild) {
       mPoint.Set(mChild);
     } else {
       // If the point referred after the last child, let's keep referring
       // after current last node of the old container.
-      mPoint.Set(mPoint.Container(), mPoint.Container()->Length());
+      mPoint.SetToEndOf(mPoint.Container());
     }
   }
 
 private:
   EditorDOMPoint& mPoint;
   // Needs to store child node by ourselves because EditorDOMPoint stores
   // child node with mRef which is previous sibling of current child node.
   // Therefore, we cannot keep referring it if it's first child.
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -1441,18 +1441,17 @@ HTMLEditRules::WillInsertText(EditAction
           nsCOMPtr<Element> br =
             mHTMLEditor->CreateBRImpl(*aSelection, currentPoint.AsRaw(),
                                       nsIEditor::eNone);
           NS_ENSURE_STATE(br);
           pos++;
           if (br->GetNextSibling()) {
             pointToInsert.Set(br->GetNextSibling());
           } else {
-            pointToInsert.Set(currentPoint.Container(),
-                              currentPoint.Container()->Length());
+            pointToInsert.SetToEndOf(currentPoint.Container());
           }
           // XXX In most cases, pointToInsert and currentPoint are same here.
           //     But if the <br> element has been moved to different point by
           //     mutation observer, those points become different.
           currentPoint.Set(br);
           DebugOnly<bool> advanced = currentPoint.AdvanceOffset();
           NS_WARNING_ASSERTION(advanced,
             "Failed to advance offset after the new <br> element");
@@ -1514,18 +1513,17 @@ HTMLEditRules::WillInsertText(EditAction
                               nsIEditor::eNone);
           if (NS_WARN_IF(!newBRElement)) {
             return NS_ERROR_FAILURE;
           }
           pos++;
           if (newBRElement->GetNextSibling()) {
             pointToInsert.Set(newBRElement->GetNextSibling());
           } else {
-            pointToInsert.Set(currentPoint.Container(),
-                              currentPoint.Container()->Length());
+            pointToInsert.SetToEndOf(currentPoint.Container());
           }
           currentPoint.Set(newBRElement);
           DebugOnly<bool> advanced = currentPoint.AdvanceOffset();
           NS_WARNING_ASSERTION(advanced,
             "Failed to advance offset to after the new <br> node");
           // XXX If the newBRElement has been moved or removed by mutation
           //     observer, we hit this assert.  We need to check if
           //     newBRElement is in expected point, though, we must have
@@ -6882,17 +6880,17 @@ HTMLEditRules::ReturnInParagraph(Selecti
     } else {
       if (doesCRCreateNewP) {
         ErrorResult error;
         nsCOMPtr<nsIContent> newLeftDivOrP =
           htmlEditor->SplitNode(pointToSplitParentDivOrP.AsRaw(), error);
         if (NS_WARN_IF(error.Failed())) {
           return EditActionResult(error.StealNSResult());
         }
-        pointToSplitParentDivOrP.Set(newLeftDivOrP, newLeftDivOrP->Length());
+        pointToSplitParentDivOrP.SetToEndOf(newLeftDivOrP);
       }
 
       // We need to put new <br> after the left node if given node was split
       // above.
       pointToInsertBR.Set(pointToSplitParentDivOrP.Container());
       DebugOnly<bool> advanced = pointToInsertBR.AdvanceOffset();
       NS_WARNING_ASSERTION(advanced,
         "Failed to advance offset to after the container of selection start");
@@ -7609,17 +7607,18 @@ HTMLEditRules::JoinNodesSmart(nsIContent
       return EditorDOMPoint();
     }
     nsresult rv = mHTMLEditor->MoveNode(&aNodeRight, parent, parOffset);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return EditorDOMPoint();
     }
   }
 
-  EditorDOMPoint ret(&aNodeRight, aNodeLeft.Length());
+  EditorDOMPoint ret;
+  ret.SetToEndOf(&aNodeRight);
 
   // Separate join rules for differing blocks
   if (HTMLEditUtils::IsList(&aNodeLeft) || aNodeLeft.GetAsText()) {
     // For lists, merge shallow (wouldn't want to combine list items)
     nsresult rv = mHTMLEditor->JoinNodes(aNodeLeft, aNodeRight);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return EditorDOMPoint();
     }
@@ -7915,17 +7914,17 @@ HTMLEditRules::PinSelectionToNewBlock(Se
     NS_ENSURE_STATE(mHTMLEditor);
     nsCOMPtr<nsINode> tmp = mHTMLEditor->GetLastEditableChild(*mNewBlock);
     if (!tmp) {
       tmp = mNewBlock;
     }
     EditorRawDOMPoint endPoint;
     if (EditorBase::IsTextNode(tmp) ||
         mHTMLEditor->IsContainer(tmp)) {
-      endPoint.Set(tmp, tmp->Length());
+      endPoint.SetToEndOf(tmp);
     } else {
       endPoint.Set(tmp);
       if (NS_WARN_IF(!endPoint.AdvanceOffset())) {
         return NS_ERROR_FAILURE;
       }
     }
     return aSelection->Collapse(endPoint);
   }
@@ -9299,18 +9298,18 @@ HTMLEditRules::WillAbsolutePosition(Sele
           return splitNodeResult.Rv();
         }
         if (!curPositionedDiv) {
           curPositionedDiv =
             htmlEditor->CreateNode(nsGkAtoms::div,
                                    splitNodeResult.SplitPoint());
           mNewBlock = curPositionedDiv;
         }
-        EditorRawDOMPoint atEndOfCurPositionedDiv(curPositionedDiv,
-                                                  curPositionedDiv->Length());
+        EditorRawDOMPoint atEndOfCurPositionedDiv;
+        atEndOfCurPositionedDiv.SetToEndOf(curPositionedDiv);
         curList =
           htmlEditor->CreateNode(containerName, atEndOfCurPositionedDiv);
         NS_ENSURE_STATE(curList);
         // curList is now the correct thing to put curNode in.  Remember our
         // new block for postprocessing.
       }
       // Tuck the node into the end of the active list
       rv = htmlEditor->MoveNode(curNode->AsContent(), curList, -1);
@@ -9349,18 +9348,18 @@ HTMLEditRules::WillAbsolutePosition(Sele
           return splitNodeResult.Rv();
         }
         if (!curPositionedDiv) {
           EditorRawDOMPoint atListItemParent(atListItem.Container());
           curPositionedDiv =
             htmlEditor->CreateNode(nsGkAtoms::div, atListItemParent);
           mNewBlock = curPositionedDiv;
         }
-        EditorRawDOMPoint atEndOfCurPositionedDiv(curPositionedDiv,
-                                                  curPositionedDiv->Length());
+        EditorRawDOMPoint atEndOfCurPositionedDiv;
+        atEndOfCurPositionedDiv.SetToEndOf(curPositionedDiv);
         curList =
           htmlEditor->CreateNode(containerName, atEndOfCurPositionedDiv);
         NS_ENSURE_STATE(curList);
       }
       rv = htmlEditor->MoveNode(listItem, curList, -1);
       NS_ENSURE_SUCCESS(rv, rv);
       // Remember we indented this li
       indentedLI = listItem;
--- a/editor/libeditor/HTMLEditorDataTransfer.cpp
+++ b/editor/libeditor/HTMLEditorDataTransfer.cpp
@@ -150,18 +150,17 @@ HTMLEditor::LoadHTML(const nsAString& aI
       // XXX If the inserted node has been moved by mutation observer,
       //     incrementing offset will cause odd result.  Next new node
       //     will be inserted after existing node and the offset will be
       //     overflown from the container node.
       pointToInsert.Set(pointToInsert.Container(), pointToInsert.Offset() + 1);
       if (NS_WARN_IF(!pointToInsert.Offset())) {
         // Append the remaining children to the container if offset is
         // overflown.
-        pointToInsert.Set(pointToInsert.Container(),
-                          pointToInsert.Container()->Length());
+        pointToInsert.SetToEndOf(pointToInsert.Container());
       }
     }
   }
 
   return rules->DidDoAction(selection, &ruleInfo, rv);
 }
 
 NS_IMETHODIMP
--- a/editor/libeditor/InsertNodeTransaction.cpp
+++ b/editor/libeditor/InsertNodeTransaction.cpp
@@ -61,26 +61,24 @@ InsertNodeTransaction::DoTransaction()
   if (!mPointToInsert.IsSetAndValid()) {
     // It seems that DOM tree has been changed after first DoTransaction()
     // and current RedoTranaction() call.
     if (mPointToInsert.GetChildAtOffset()) {
       EditorDOMPoint newPointToInsert(mPointToInsert.GetChildAtOffset());
       if (!newPointToInsert.IsSet()) {
         // The insertion point has been removed from the DOM tree.
         // In this case, we should append the node to the container instead.
-        newPointToInsert.Set(mPointToInsert.Container(),
-                             mPointToInsert.Container()->Length());
+        newPointToInsert.SetToEndOf(mPointToInsert.Container());
         if (NS_WARN_IF(!newPointToInsert.IsSet())) {
           return NS_ERROR_FAILURE;
         }
       }
       mPointToInsert = newPointToInsert;
     } else {
-      mPointToInsert.Set(mPointToInsert.Container(),
-                         mPointToInsert.Container()->Length());
+      mPointToInsert.SetToEndOf(mPointToInsert.Container());
       if (NS_WARN_IF(!mPointToInsert.IsSet())) {
         return NS_ERROR_FAILURE;
       }
     }
   }
 
   mEditorBase->MarkNodeDirty(GetAsDOMNode(mContentToInsert));
 
--- a/editor/libeditor/SplitNodeTransaction.cpp
+++ b/editor/libeditor/SplitNodeTransaction.cpp
@@ -85,17 +85,18 @@ SplitNodeTransaction::DoTransaction()
     if (NS_WARN_IF(!selection)) {
       return NS_ERROR_FAILURE;
     }
     if (NS_WARN_IF(error.Failed())) {
       // XXX This must be a bug.
       error.SuppressException();
     }
     MOZ_ASSERT(mStartOfRightNode.Offset() == mNewLeftNode->Length());
-    EditorRawDOMPoint atEndOfLeftNode(mNewLeftNode, mNewLeftNode->Length());
+    EditorRawDOMPoint atEndOfLeftNode;
+    atEndOfLeftNode.SetToEndOf(mNewLeftNode);
     selection->Collapse(atEndOfLeftNode, error);
   }
 
   if (NS_WARN_IF(error.Failed())) {
     return error.StealNSResult();
   }
   return NS_OK;
 }