Bug 1423835 - part 1: Add EditorDOMPointBase::SetToEndOf() to initialize the instance at end of container node r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 07 Dec 2017 17:27:20 +0900
changeset 395970 1427b2fca8d7453a4f46be04981db274dd6abe36
parent 395969 ec4df7b61cce17758ac02acc834a4e8c7dbc0767
child 395971 c41a81c807691ef4c945895e0efa64d0e8768086
push id56869
push usermasayuki@d-toybox.com
push dateSun, 10 Dec 2017 06:01:14 +0000
treeherderautoland@92403ed2a488 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1423835
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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;
 }