Bug 1413181 - part 7: EditorBase::SplitNodeDeep() should stop splitting orphan node if it meets an orphan node before meeting the most ancestor node to be split r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 13 Nov 2017 15:38:23 +0900
changeset 701127 a31409c57a0014234c685334a608fd5057dad4a4
parent 701126 14bf477a5041d6701f065d7f2f1d96c0aed304a3
child 701128 f0bcbddc994e0b05203439d4adb8b76b879ed45b
push id90079
push usermasayuki@d-toybox.com
push dateTue, 21 Nov 2017 08:41:26 +0000
reviewersm_kato
bugs1413181
milestone59.0a1
Bug 1413181 - part 7: EditorBase::SplitNodeDeep() should stop splitting orphan node if it meets an orphan node before meeting the most ancestor node to be split r?m_kato This doesn't change any meaning of the loop. It is a bug if the loop meets orphan node before meeting the most ancestor node to be split which is given as aNode. So, we can check it before trying to split it. MozReview-Commit-ID: 1TD7WHCoZh1
editor/libeditor/EditorBase.cpp
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -4067,16 +4067,23 @@ EditorBase::SplitNodeDeep(nsIContent& aN
 {
   MOZ_ASSERT(&aSplitPointParent == &aNode ||
              EditorUtils::IsDescendantOf(aSplitPointParent, aNode));
   int32_t offset = aSplitPointOffset;
 
   nsCOMPtr<nsIContent> leftNode, rightNode;
   OwningNonNull<nsIContent> nodeToSplit = aSplitPointParent;
   while (true) {
+    // If we meet an orphan node before meeting aNode, we need to stop
+    // splitting.  This is a bug of the caller.
+    nsCOMPtr<nsIContent> parent = nodeToSplit->GetParent();
+    if (NS_WARN_IF(nodeToSplit != &aNode && !parent)) {
+      return -1;
+    }
+
     // Need to insert rules code call here to do things like not split a list
     // if you are after the last <li> or before the first, etc.  For now we
     // just have some smarts about unneccessarily splitting text nodes, which
     // should be universal enough to put straight in this EditorBase routine.
 
     bool didSplit = false;
 
     if ((aSplitAtEdges == SplitAtEdges::eAllowToCreateEmptyContainer &&
@@ -4094,34 +4101,31 @@ EditorBase::SplitNodeDeep(nsIContent& aN
         error.SuppressException();
         return -1;
       }
 
       rightNode = nodeToSplit;
       leftNode = newLeftNode;
     }
 
-    NS_ENSURE_TRUE(nodeToSplit->GetParent(), -1);
-    OwningNonNull<nsIContent> parentNode = *nodeToSplit->GetParent();
-
     if (!didSplit && offset) {
       // Must be "end of text node" case, we didn't split it, just move past it
-      offset = parentNode->IndexOf(nodeToSplit) + 1;
+      offset = parent->IndexOf(nodeToSplit) + 1;
       leftNode = nodeToSplit;
     } else {
-      offset = parentNode->IndexOf(nodeToSplit);
+      offset = parent->IndexOf(nodeToSplit);
       rightNode = nodeToSplit;
     }
 
     if (nodeToSplit == &aNode) {
       // we split all the way up to (and including) aNode; we're done
       break;
     }
 
-    nodeToSplit = parentNode;
+    nodeToSplit = *parent;
   }
 
   if (aOutLeftNode) {
     leftNode.forget(aOutLeftNode);
   }
   if (aOutRightNode) {
     rightNode.forget(aOutRightNode);
   }