Bug 1408125 - part 5: Redesign HTMLEditor::NormalizeEOLInsertPosition() with EditorRawDOMPoint r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 05 Dec 2017 15:36:49 +0900
changeset 710428 68f119a84599b19d560132a0ad2de0b4a43aac54
parent 710427 cd51d1fe8ac3b118e8c38b415ae94de8374d2c26
child 710429 eb79f2088052a70b21638f4a3852fae81f7cb989
push id92823
push usermasayuki@d-toybox.com
push dateSun, 10 Dec 2017 02:40:55 +0000
reviewersm_kato
bugs1408125
milestone59.0a1
Bug 1408125 - part 5: Redesign HTMLEditor::NormalizeEOLInsertPosition() with EditorRawDOMPoint r?m_kato HTMLEditor::NormalizeEOLInsertPosition() takes a set of container node and offset in it for specifying insertion point. So, this should be replaced with |const EditorRawDOMPoint&| and it should return |EditorDOMPoint| rather than modifying the argument. Additionally, perhaps, GetBetterInsertionPointFor() is better name for it. MozReview-Commit-ID: IB1FhrkzK2G
editor/libeditor/EditorDOMPoint.h
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditor.h
editor/libeditor/HTMLEditorDataTransfer.cpp
--- a/editor/libeditor/EditorDOMPoint.h
+++ b/editor/libeditor/EditorDOMPoint.h
@@ -153,17 +153,17 @@ public:
                                              nullptr))
     , mOffset(aOther.mOffset)
     , mIsChildInitialized(aOther.mRef ||
                           (aOther.mOffset.isSome() && !aOther.mOffset.value()))
   {
   }
 
   template<typename PT, typename CT>
-  explicit EditorDOMPointBase(const EditorDOMPointBase<PT, CT>& aOther)
+  MOZ_IMPLICIT EditorDOMPointBase(const EditorDOMPointBase<PT, CT>& aOther)
     : mParent(aOther.mParent)
     , mChild(aOther.mChild)
     , mOffset(aOther.mOffset)
     , mIsChildInitialized(aOther.mIsChildInitialized)
   {
   }
 
   // Following methods are just copy of same methods of RangeBoudnaryBase.
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -1351,90 +1351,74 @@ HTMLEditor::RebuildDocumentFromSource(co
 
   // Copy all attributes from the div child to current body element
   CloneAttributes(bodyElement, child->AsElement());
 
   // place selection at first editable content
   return BeginningOfDocument();
 }
 
-void
-HTMLEditor::NormalizeEOLInsertPosition(nsINode* firstNodeToInsert,
-                                       nsCOMPtr<nsIDOMNode>* insertParentNode,
-                                       int32_t* insertOffset)
+EditorRawDOMPoint
+HTMLEditor::GetBetterInsertionPointFor(nsINode& aNodeToInsert,
+                                       const EditorRawDOMPoint& aPointToInsert)
 {
-  /*
-    This function will either correct the position passed in,
-    or leave the position unchanged.
-
-    When the (first) item to insert is a block level element,
-    and our insertion position is after the last visible item in a line,
-    i.e. the insertion position is just before a visible line break <br>,
-    we want to skip to the position just after the line break (see bug 68767)
-
-    However, our logic to detect whether we should skip or not
-    needs to be more clever.
-    We must not skip when the caret appears to be positioned at the beginning
-    of a block, in that case skipping the <br> would not insert the <br>
-    at the caret position, but after the current empty line.
-
-    So we have several cases to test:
-
-    1) We only ever want to skip, if the next visible thing after the current position is a break
-
-    2) We do not want to skip if there is no previous visible thing at all
-       That is detected if the call to PriorVisibleNode gives us an offset of zero.
-       Because PriorVisibleNode always positions after the prior node, we would
-       see an offset > 0, if there were a prior node.
-
-    3) We do not want to skip, if both the next and the previous visible things are breaks.
-
-    4) We do not want to skip if the previous visible thing is in a different block
-       than the insertion position.
-  */
-
-  if (!IsBlockNode(firstNodeToInsert)) {
-    return;
-  }
-
-  WSRunObject wsObj(this, *insertParentNode, *insertOffset);
-  nsCOMPtr<nsINode> nextVisNode, prevVisNode;
-  int32_t nextVisOffset=0;
-  WSType nextVisType;
-  int32_t prevVisOffset=0;
-  WSType prevVisType;
-
-  nsCOMPtr<nsINode> parent(do_QueryInterface(*insertParentNode));
-  wsObj.NextVisibleNode(parent, *insertOffset, address_of(nextVisNode), &nextVisOffset, &nextVisType);
-  if (!nextVisNode) {
-    return;
-  }
-
-  if (!(nextVisType & WSType::br)) {
-    return;
-  }
-
-  wsObj.PriorVisibleNode(parent, *insertOffset, address_of(prevVisNode), &prevVisOffset, &prevVisType);
-  if (!prevVisNode) {
-    return;
-  }
-
-  if (prevVisType & WSType::br) {
-    return;
-  }
-
-  if (prevVisType & WSType::thisBlock) {
-    return;
-  }
-
-  int32_t brOffset=0;
-  nsCOMPtr<nsIDOMNode> brNode = GetNodeLocation(GetAsDOMNode(nextVisNode), &brOffset);
-
-  *insertParentNode = brNode;
-  *insertOffset = brOffset + 1;
+  if (NS_WARN_IF(!aPointToInsert.IsSet())) {
+    return aPointToInsert;
+  }
+
+  // If the node to insert is not a block level element, we can insert it
+  // at any point.
+  if (!IsBlockNode(&aNodeToInsert)) {
+    return aPointToInsert;
+  }
+
+  WSRunObject wsObj(this, aPointToInsert.Container(), aPointToInsert.Offset());
+
+  // If the insertion position is after the last visible item in a line,
+  // i.e., the insertion position is just before a visible line break <br>,
+  // we want to skip to the position just after the line break (see bug 68767).
+  nsCOMPtr<nsINode> nextVisibleNode;
+  int32_t nextVisibleOffset = 0;
+  WSType nextVisibleType;
+  wsObj.NextVisibleNode(aPointToInsert.Container(), aPointToInsert.Offset(),
+                        address_of(nextVisibleNode),
+                        &nextVisibleOffset, &nextVisibleType);
+  // So, if the next visible node isn't a <br> element, we can insert the block
+  // level element to the point.
+  if (!nextVisibleNode ||
+      !(nextVisibleType & WSType::br)) {
+    return aPointToInsert;
+  }
+
+  // However, we must not skip next <br> element when the caret appears to be
+  // positioned at the beginning of a block, in that case skipping the <br>
+  // would not insert the <br> at the caret position, but after the current
+  // empty line.
+  nsCOMPtr<nsINode> previousVisibleNode;
+  int32_t previousVisibleOffset = 0;
+  WSType previousVisibleType;
+  wsObj.PriorVisibleNode(aPointToInsert.Container(), aPointToInsert.Offset(),
+                         address_of(previousVisibleNode),
+                         &previousVisibleOffset, &previousVisibleType);
+  // So, if there is no previous visible node,
+  // or, if both nodes of the insertion point is <br> elements,
+  // or, if the previous visible node is different block,
+  // we need to skip the following <br>.  So, otherwise, we can insert the
+  // block at the insertion point.
+  if (!previousVisibleNode ||
+      (previousVisibleType & WSType::br) ||
+      (previousVisibleType & WSType::thisBlock)) {
+    return aPointToInsert;
+  }
+
+  EditorRawDOMPoint afterBRNode(nextVisibleNode);
+  DebugOnly<bool> advanced = afterBRNode.AdvanceOffset();
+  NS_WARNING_ASSERTION(advanced,
+    "Failed to advance offset to after the <br> node");
+  return afterBRNode;
 }
 
 NS_IMETHODIMP
 HTMLEditor::InsertElementAtSelection(nsIDOMElement* aElement,
                                      bool aDeleteSelection)
 {
   // Protect the edit rules object from dying
   nsCOMPtr<nsIEditRules> rules(mRules);
@@ -1485,29 +1469,33 @@ HTMLEditor::InsertElementAtSelection(nsI
       // For all other tags, we insert AFTER the selection
       if (HTMLEditUtils::IsNamedAnchor(node)) {
         selection->CollapseToStart();
       } else {
         selection->CollapseToEnd();
       }
     }
 
-    nsINode* parentSelectedNode = selection->GetAnchorNode();
-    if (parentSelectedNode) {
-      int32_t offsetForInsert = selection->AnchorOffset();
+    if (selection->GetAnchorNode()) {
+      EditorRawDOMPoint atAnchor;
+      if (selection->GetChildAtAnchorOffset()) {
+        atAnchor.Set(selection->GetChildAtAnchorOffset());
+      } else {
+        atAnchor.Set(selection->GetAnchorNode(), selection->AnchorOffset());
+      }
       // Adjust position based on the node we are going to insert.
-      nsCOMPtr<nsIDOMNode> parentSelectedDOMNode =
-        GetAsDOMNode(parentSelectedNode);
-      NormalizeEOLInsertPosition(element, address_of(parentSelectedDOMNode),
-                                 &offsetForInsert);
+      EditorRawDOMPoint pointToInsert =
+        GetBetterInsertionPointFor(*element, atAnchor);
+      if (NS_WARN_IF(!pointToInsert.IsSet())) {
+        return NS_ERROR_FAILURE;
+      }
 
       EditorDOMPoint insertedPoint =
         InsertNodeIntoProperAncestor(
-          *element, EditorRawDOMPoint(parentSelectedDOMNode, offsetForInsert),
-          SplitAtEdges::eAllowToCreateEmptyContainer);
+          *element, pointToInsert, SplitAtEdges::eAllowToCreateEmptyContainer);
       if (NS_WARN_IF(!insertedPoint.IsSet())) {
         return NS_ERROR_FAILURE;
       }
       // Set caret after element, but check for special case
       //  of inserting table-related elements: set in first cell instead
       if (!SetCaretInTableCell(aElement)) {
         rv = SetCaretAfterElement(aElement);
         NS_ENSURE_SUCCESS(rv, rv);
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -703,22 +703,29 @@ protected:
          int32_t aHighWaterMark);
 
   /**
    * Small utility routine to test if a break node is visible to user.
    */
   bool IsVisibleBRElement(nsINode* aNode);
 
   /**
-   * Utility routine to possibly adjust the insertion position when
-   * inserting a block level element.
+   * GetBetterInsertionPointFor() returns better insertion point to insert
+   * aNodeToInsert.
+   *
+   * @param aNodeToInsert       The node to insert.
+   * @param aPointToInsert      A candidate point to insert the node.
+   * @return                    Better insertion point if next visible node
+   *                            is a <br> element and previous visible node
+   *                            is neither none, another <br> element nor
+   *                            different block level element.
    */
-  void NormalizeEOLInsertPosition(nsINode* firstNodeToInsert,
-                                  nsCOMPtr<nsIDOMNode>* insertParentNode,
-                                  int32_t* insertOffset);
+  EditorRawDOMPoint
+  GetBetterInsertionPointFor(nsINode& aNodeToInsert,
+                             const EditorRawDOMPoint& aPointToInsert);
 
   /**
    * Helpers for block transformations.
    */
   nsresult MakeDefinitionItem(const nsAString& aItemType);
   nsresult InsertBasicBlock(const nsAString& aBlockType);
 
   /**
--- a/editor/libeditor/HTMLEditorDataTransfer.cpp
+++ b/editor/libeditor/HTMLEditorDataTransfer.cpp
@@ -339,61 +339,52 @@ HTMLEditor::DoInsertHTMLWithContext(cons
   bool cancel, handled;
   rv = rules->WillDoAction(selection, &ruleInfo, &cancel, &handled);
   NS_ENSURE_SUCCESS(rv, rv);
   if (cancel) {
     return NS_OK; // rules canceled the operation
   }
 
   if (!handled) {
-    // The rules code (WillDoAction above) might have changed the selection.
-    // refresh our memory...
-    nsCOMPtr<nsIDOMNode> parentNode;
-    int32_t offsetOfNewNode;
-    rv = GetStartNodeAndOffset(selection, getter_AddRefs(parentNode), &offsetOfNewNode);
-    NS_ENSURE_SUCCESS(rv, rv);
-    NS_ENSURE_TRUE(parentNode, NS_ERROR_FAILURE);
-
     // Adjust position based on the first node we are going to insert.
-    NormalizeEOLInsertPosition(nodeList[0], address_of(parentNode),
-                               &offsetOfNewNode);
+    // FYI: WillDoAction() above might have changed the selection.
+    EditorDOMPoint pointToInsert =
+      GetBetterInsertionPointFor(nodeList[0], GetStartPoint(selection));
+    if (NS_WARN_IF(!pointToInsert.IsSet())) {
+      return NS_ERROR_FAILURE;
+    }
 
     // if there are any invisible br's after our insertion point, remove them.
     // this is because if there is a br at end of what we paste, it will make
     // the invisible br visible.
-    WSRunObject wsObj(this, parentNode, offsetOfNewNode);
+    WSRunObject wsObj(this, pointToInsert.Container(), pointToInsert.Offset());
     if (wsObj.mEndReasonNode &&
         TextEditUtils::IsBreak(wsObj.mEndReasonNode) &&
         !IsVisibleBRElement(wsObj.mEndReasonNode)) {
+      AutoEditorDOMPointChildInvalidator lockOffset(pointToInsert);
       rv = DeleteNode(wsObj.mEndReasonNode);
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     // Remember if we are in a link.
-    bool bStartedInLink = IsInLink(parentNode);
+    bool bStartedInLink = IsInLink(pointToInsert.Container()->AsDOMNode());
 
     // Are we in a text node? If so, split it.
-    if (IsTextNode(parentNode)) {
-      nsCOMPtr<nsIContent> parentContent = do_QueryInterface(parentNode);
-      EditorRawDOMPoint pointToSplit(parentContent, offsetOfNewNode);
-      if (NS_WARN_IF(!pointToSplit.IsSet())) {
-        return NS_ERROR_FAILURE;
-      }
+    if (IsTextNode(pointToInsert.Container())) {
       SplitNodeResult splitNodeResult =
-        SplitNodeDeep(*parentContent, pointToSplit,
+        SplitNodeDeep(*pointToInsert.Container()->AsContent(),
+                      pointToInsert.AsRaw(),
                       SplitAtEdges::eAllowToCreateEmptyContainer);
       if (NS_WARN_IF(splitNodeResult.Failed())) {
         return splitNodeResult.Rv();
       }
-      EditorRawDOMPoint splitPoint(splitNodeResult.SplitPoint());
-      if (NS_WARN_IF(!splitPoint.IsSet())) {
+      pointToInsert = splitNodeResult.SplitPoint();
+      if (NS_WARN_IF(!pointToInsert.IsSet())) {
         return NS_ERROR_FAILURE;
       }
-      parentNode = do_QueryInterface(splitPoint.Container());
-      offsetOfNewNode = splitPoint.Offset();
     }
 
     // build up list of parents of first node in list that are either
     // lists or tables.  First examine front of paste node list.
     nsTArray<OwningNonNull<Element>> startListAndTableArray;
     GetListAndTableParents(StartOrEnd::start, nodeList,
                            startListAndTableArray);
 
@@ -424,29 +415,26 @@ HTMLEditor::DoInsertHTMLWithContext(cons
     }
 
     // don't orphan partial list or table structure
     if (highWaterMark >= 0) {
       ReplaceOrphanedStructure(StartOrEnd::end, nodeList,
                                endListAndTableArray, highWaterMark);
     }
 
+    MOZ_ASSERT(pointToInsert.Container()->GetChildAt(pointToInsert.Offset()) ==
+                 pointToInsert.GetChildAtOffset());
+
     // Loop over the node list and paste the nodes:
-    nsCOMPtr<nsIDOMNode> parentBlock;
-    nsCOMPtr<nsINode> parentNodeNode = do_QueryInterface(parentNode);
-    NS_ENSURE_STATE(parentNodeNode || !parentNode);
-    if (IsBlockNode(parentNodeNode)) {
-      parentBlock = parentNode;
-    } else if (parentNodeNode) {
-      parentBlock = GetAsDOMNode(GetBlockNodeParent(parentNodeNode));
-    }
-
+    nsCOMPtr<nsINode> parentBlock =
+      IsBlockNode(pointToInsert.Container()) ?
+        pointToInsert.Container() :
+        GetBlockNodeParent(pointToInsert.Container());
     nsCOMPtr<nsIContent> lastInsertNode;
     nsCOMPtr<nsINode> insertedContextParent;
-    EditorDOMPoint pointToInsert(parentNodeNode, offsetOfNewNode);
     for (OwningNonNull<nsINode>& curNode : nodeList) {
       if (NS_WARN_IF(curNode == fragmentAsNodeNode) ||
           NS_WARN_IF(TextEditUtils::IsBody(curNode))) {
         return NS_ERROR_FAILURE;
       }
 
       if (insertedContextParent) {
         // If we had to insert something higher up in the paste hierarchy,