Bug 1685303: part 16) Slightly simplify code in `nsFrameSelection::TakeFocus` and add logging. r=smaug

Depends on D101931

Differential Revision: https://phabricator.services.mozilla.com/D101932

layout/generic/nsFrameSelection.cpp

@@ -109,8 +109,7 @@ static nsresult AddCellsToSelection(const nsIContent* aTableContent,
                                     Selection& aNormalSelection);
 
 static nsAtom* GetTag(nsINode* aNode);
-// returns the parent
+
-static nsINode* ParentOffset(nsINode* aNode, int32_t* aChildOffset);
 static nsINode* GetClosestInclusiveTableCellAncestor(nsINode* aDomNode);
 MOZ_CAN_RUN_SCRIPT_BOUNDARY static nsresult CreateAndAddRange(
     nsINode* aContainer, int32_t aOffset, Selection& aNormalSelection);
@@ -654,20 +653,6 @@ static nsAtom* GetTag(nsINode* aNode) {
   return content->NodeInfo()->NameAtom();
 }
 
-// Returns the parent
-nsINode* ParentOffset(nsINode* aNode, int32_t* aChildOffset) {
-  if (!aNode || !aChildOffset) return nullptr;
-
-  nsIContent* parent = aNode->GetParent();
-  if (parent) {
-    *aChildOffset = parent->ComputeIndexOf(aNode);
-
-    return parent;
-  }
-
-  return nullptr;
-}
-
 /**
  * https://dom.spec.whatwg.org/#concept-tree-inclusive-ancestor.
  */
@@ -1344,6 +1329,19 @@ nsINode* nsFrameSelection::TableSelection::IsContentInActivelyEditableTableCell(
   return editableCell ? inclusiveTableCellAncestor : nullptr;
 }
 
+namespace {
+struct ParentAndOffset {
+  explicit ParentAndOffset(const nsINode& aNode)
+      : mParent{aNode.GetParent()},
+        mOffset{mParent ? mParent->ComputeIndexOf(&aNode) : 0} {}
+
+  nsINode* mParent;
+
+  // 0, if there's no parent.
+  int32_t mOffset;
+};
+
+}  // namespace
 /**
 hard to go from nodes to frames, easy the other way!
  */
@@ -1362,6 +1360,11 @@ nsresult nsFrameSelection::TakeFocus(nsIContent* const aNewFocus,
     return NS_ERROR_FAILURE;
   }
 
+  MOZ_LOG(sFrameSelectionLog, LogLevel::Verbose,
+          ("%s: new focus=%p, offsets=(%u, %u), hint=%i, focusMode=%i",
+           __FUNCTION__, aNewFocus, aContentOffset, aContentEndOffset,
+           static_cast<int>(aHint), static_cast<int>(aFocusMode)));
+
   mPresShell->FrameSelectionWillTakeFocus(*this);
 
   // Clear all table selection data
@@ -1405,7 +1408,6 @@ nsresult nsFrameSelection::TakeFocus(nsIContent* const aNewFocus,
         const RefPtr<Selection> selection{mDomSelections[index]};
         selection->AddRangeAndSelectFramesAndNotifyListeners(*newRange,
                                                              IgnoreErrors());
-        mBatching = saveBatching;
       } else {
         bool oldDesiredPosSet =
             mDesiredCaretPos.mIsSet;  // need to keep old desired
@@ -1414,8 +1416,10 @@ nsresult nsFrameSelection::TakeFocus(nsIContent* const aNewFocus,
         selection->CollapseInLimiter(aNewFocus, aContentOffset);
         mDesiredCaretPos.mIsSet =
             oldDesiredPosSet;  // now reset desired pos back.
-        mBatching = saveBatching;
       }
+
+      mBatching = saveBatching;
+
       if (aContentEndOffset != aContentOffset) {
         mDomSelections[index]->Extend(aNewFocus, aContentEndOffset);
       }
@@ -1433,16 +1437,14 @@ nsresult nsFrameSelection::TakeFocus(nsIContent* const aNewFocus,
                                                                    aNewFocus)) {
         mTableSelection.mClosestInclusiveTableCellAncestor =
             inclusiveTableCellAncestor;
-#ifdef DEBUG_TABLE_SELECTION
+        MOZ_LOG(sFrameSelectionLog, LogLevel::Debug,
-        printf(" * TakeFocus - Collapsing into new cell\n");
+                ("%s: Collapsing into new cell", __FUNCTION__));
-#endif
       }
 
       break;
     }
     case FocusMode::kExtendSelection: {
       // Now update the range list:
-      int32_t offset;
       nsINode* inclusiveTableCellAncestor =
           GetClosestInclusiveTableCellAncestor(aNewFocus);
       if (mTableSelection.mClosestInclusiveTableCellAncestor &&
@@ -1452,35 +1454,37 @@ nsresult nsFrameSelection::TakeFocus(nsIContent* const aNewFocus,
                   .mClosestInclusiveTableCellAncestor)  // switch to cell
                                                         // selection mode
       {
-#ifdef DEBUG_TABLE_SELECTION
+        MOZ_LOG(sFrameSelectionLog, LogLevel::Debug,
-        printf(" * TakeFocus - moving into new cell\n");
+                ("%s: moving into new cell", __FUNCTION__));
-#endif
+
         WidgetMouseEvent event(false, eVoidEvent, nullptr,
                                WidgetMouseEvent::eReal);
 
         // Start selecting in the cell we were in before
-        nsINode* parent = ParentOffset(
+        ParentAndOffset parentAndOffset{
-            mTableSelection.mClosestInclusiveTableCellAncestor, &offset);
+            *mTableSelection.mClosestInclusiveTableCellAncestor};
-        if (parent) {
+        if (parentAndOffset.mParent) {
           const nsresult result = HandleTableSelection(
-              parent, offset, TableSelectionMode::Cell, &event);
+              parentAndOffset.mParent, parentAndOffset.mOffset,
+              TableSelectionMode::Cell, &event);
           if (NS_WARN_IF(NS_FAILED(result))) {
             return result;
           }
         }
 
         // Find the parent of this new cell and extend selection to it
-        parent = ParentOffset(inclusiveTableCellAncestor, &offset);
+        parentAndOffset = ParentAndOffset{*inclusiveTableCellAncestor};
 
         // XXXX We need to REALLY get the current key shift state
         //  (we'd need to add event listener -- let's not bother for now)
         event.mModifiers &= ~MODIFIER_SHIFT;  // aContinueSelection;
-        if (parent) {
+        if (parentAndOffset.mParent) {
           mTableSelection.mClosestInclusiveTableCellAncestor =
               inclusiveTableCellAncestor;
           // Continue selection into next cell
           const nsresult result = HandleTableSelection(
-              parent, offset, TableSelectionMode::Cell, &event);
+              parentAndOffset.mParent, parentAndOffset.mOffset,
+              TableSelectionMode::Cell, &event);
           if (NS_WARN_IF(NS_FAILED(result))) {
             return result;
           }