From af17c6956b101be4a81059d7b64d3d35f97edfc6 Mon Sep 17 00:00:00 2001 From: Mirko Brodesser Date: Fri, 15 Jan 2021 16:00:20 +0000 Subject: [PATCH] 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 | 68 +++++++++++++++-------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/layout/generic/nsFrameSelection.cpp b/layout/generic/nsFrameSelection.cpp index 6e9f95480630..7adedc935535 100644 --- a/layout/generic/nsFrameSelection.cpp +++ b/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(aHint), static_cast(aFocusMode))); + mPresShell->FrameSelectionWillTakeFocus(*this); // Clear all table selection data @@ -1405,7 +1408,6 @@ nsresult nsFrameSelection::TakeFocus(nsIContent* const aNewFocus, const RefPtr 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 - printf(" * TakeFocus - Collapsing into new cell\n"); -#endif + MOZ_LOG(sFrameSelectionLog, LogLevel::Debug, + ("%s: Collapsing into new cell", __FUNCTION__)); } 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 - printf(" * TakeFocus - moving into new cell\n"); -#endif + MOZ_LOG(sFrameSelectionLog, LogLevel::Debug, + ("%s: moving into new cell", __FUNCTION__)); + WidgetMouseEvent event(false, eVoidEvent, nullptr, WidgetMouseEvent::eReal); // Start selecting in the cell we were in before - nsINode* parent = ParentOffset( - mTableSelection.mClosestInclusiveTableCellAncestor, &offset); - if (parent) { + ParentAndOffset parentAndOffset{ + *mTableSelection.mClosestInclusiveTableCellAncestor}; + 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; }