Bug 1316302 part.5 Minimize variable scopes in HTMLEditRules::TryToJoinBlocks() r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 16 Nov 2016 13:08:50 +0900
changeset 439605 e541e365f1426b5547fade323fd36c5f3cbf9984
parent 439604 3f709be90884ffa67f20812e90bd9c1e83e5a5a3
child 537221 73f9e532497a51ced506c015efae821bdd5e33c8
push id36058
push usermasayuki@d-toybox.com
push dateWed, 16 Nov 2016 11:23:22 +0000
reviewerssmaug
bugs1316302
milestone53.0a1
Bug 1316302 part.5 Minimize variable scopes in HTMLEditRules::TryToJoinBlocks() r?smaug Because of using early return style in HTMLEditRules::TryToJoinBlocks(), we can minimize the scope of EditActionResult variable in it. MozReview-Commit-ID: jXVolNw41a
editor/libeditor/HTMLEditRules.cpp
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -2644,17 +2644,16 @@ HTMLEditRules::TryToJoinBlocks(nsIConten
 
   AutoTransactionsConserveSelection dontSpazMySelection(htmlEditor);
 
   int32_t rightOffset = 0;
   int32_t leftOffset = -1;
 
   // offset below is where you find yourself in rightBlock when you traverse
   // upwards from leftBlock
-  EditActionResult ret(NS_OK);
   if (EditorUtils::IsDescendantOf(leftBlock, rightBlock, &rightOffset)) {
     // Tricky case.  Left block is inside right block.  Do ws adjustment.  This
     // just destroys non-visible ws at boundaries we will be joining.
     rightOffset++;
     nsresult rv = WSRunObject::ScrubBlockBoundary(htmlEditor,
                                                   WSRunObject::kBlockEnd,
                                                   leftBlock);
     if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -2689,34 +2688,30 @@ HTMLEditRules::TryToJoinBlocks(nsIConten
       // The idea here is to take all children in rightList that are past
       // offset, and pull them into leftlist.
       for (nsCOMPtr<nsIContent> child = rightList->GetChildAt(offset);
            child; child = rightList->GetChildAt(rightOffset)) {
         rv = htmlEditor->MoveNode(child, leftList, -1);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return EditActionIgnored(rv);
         }
-      }
-      // XXX Should this set to true only when above for loop moves the node?
-      ret.MarkAsHandled();
+        // XXX Should we mark handled only when we reach here?
+      }
       if (brNode) {
         htmlEditor->DeleteNode(brNode);
       }
-      return ret;
-    }
+      return EditActionHandled();
+    }
+
     // XXX Why do we ignore the result of MoveBlock()?
     EditActionResult retMoveBlock =
       MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
-    if (retMoveBlock.Handled()) {
-      ret.MarkAsHandled();
-    }
-    if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) {
-      ret.MarkAsHandled();
-    }
-    return ret;
+    bool brRemoved = brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode));
+    return retMoveBlock.Handled() || brRemoved ?
+             EditActionHandled() : EditActionIgnored();
   }
 
   // Offset below is where you find yourself in leftBlock when you traverse
   // upwards from rightBlock
   if (EditorUtils::IsDescendantOf(rightBlock, leftBlock, &leftOffset)) {
     // Tricky case.  Right block is inside left block.  Do ws adjustment.  This
     // just destroys non-visible ws at boundaries we will be joining.
     nsresult rv = WSRunObject::ScrubBlockBoundary(htmlEditor,
@@ -2750,23 +2745,20 @@ HTMLEditRules::TryToJoinBlocks(nsIConten
     }
     // Do br adjustment.
     nsCOMPtr<Element> brNode =
       CheckForInvisibleBR(*leftBlock, BRLocation::beforeBlock, leftOffset);
     if (mergeLists) {
       // XXX Why do we ignore the result of MoveContents()?
       EditActionResult retMoveContents =
         MoveContents(*rightList, *leftList, &leftOffset);
-      if (retMoveContents.Handled()) {
-        ret.MarkAsHandled();
-      }
       if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) {
-        ret.MarkAsHandled();
-      }
-      return ret;
+        retMoveContents.MarkAsHandled();
+      }
+      return retMoveContents;
     }
 
     // Left block is a parent of right block, and the parent of the previous
     // visible content.  Right block is a child and contains the contents we
     // want to move.
 
     int32_t previousContentOffset;
     nsCOMPtr<nsINode> previousContentParent;
@@ -2817,68 +2809,72 @@ HTMLEditRules::TryToJoinBlocks(nsIConten
           previousContentParent->IndexOf(splittedPreviousContent) : -1;
       }
     }
 
     if (NS_WARN_IF(!previousContentParent)) {
       return EditActionIgnored(NS_ERROR_NULL_POINTER);
     }
 
-    ret |= MoveBlock(*previousContentParent->AsElement(), *rightBlock,
-                     previousContentOffset, rightOffset);
-    if (NS_WARN_IF(ret.Failed())) {
-      return ret;
+    EditActionResult retMoveBlock =
+      MoveBlock(*previousContentParent->AsElement(), *rightBlock,
+                previousContentOffset, rightOffset);
+    if (NS_WARN_IF(retMoveBlock.Failed())) {
+      return retMoveBlock;
     }
     if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) {
-      ret.MarkAsHandled();
-    }
-    return ret;
+      retMoveBlock.MarkAsHandled();
+    }
+    return retMoveBlock;
   }
 
   // Normal case.  Blocks are siblings, or at least close enough.  An example
   // of the latter is <p>paragraph</p><ul><li>one<li>two<li>three</ul>.  The
   // first li and the p are not true siblings, but we still want to join them
   // if you backspace from li into p.
 
   // Adjust whitespace at block boundaries
   nsresult rv =
     WSRunObject::PrepareToJoinBlocks(htmlEditor, leftBlock, rightBlock);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return EditActionIgnored(rv);
   }
   // Do br adjustment.
+  bool handled = false;
   nsCOMPtr<Element> brNode =
     CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd);
   if (mergeLists || leftBlock->NodeInfo()->NameAtom() ==
                     rightBlock->NodeInfo()->NameAtom()) {
     // Nodes are same type.  merge them.
     EditorDOMPoint pt = JoinNodesSmart(*leftBlock, *rightBlock);
     if (pt.node && mergeLists) {
       nsCOMPtr<Element> newBlock;
       ConvertListType(rightBlock, getter_AddRefs(newBlock),
                       existingList, nsGkAtoms::li);
     }
-    ret.MarkAsHandled();
+    handled = true;
   } else {
     // Nodes are dissimilar types.
-    ret |= MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
-    if (NS_WARN_IF(ret.Failed())) {
-      return ret;
-    }
+    EditActionResult retMoveBlock =
+      MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
+    if (NS_WARN_IF(retMoveBlock.Failed())) {
+      return retMoveBlock;
+    }
+    handled = retMoveBlock.Handled();
   }
   if (brNode) {
     rv = htmlEditor->DeleteNode(brNode);
     // XXX In other top level if blocks, the result of DeleteNode()
     //     is ignored.  Why does only this result is respected?
     if (NS_WARN_IF(NS_FAILED(rv))) {
-      return ret.SetResult(rv);
-    }
-    ret.MarkAsHandled();
-  }
-  return ret;
+      return handled ? EditActionHandled(rv) : EditActionIgnored(rv);
+    }
+    handled = true;
+  }
+  return handled ? EditActionHandled() : EditActionIgnored();
 }
 
 EditActionResult
 HTMLEditRules::MoveBlock(Element& aLeftBlock,
                          Element& aRightBlock,
                          int32_t aLeftOffset,
                          int32_t aRightOffset)
 {