commit 6eed70821ee6c71d34917751fc406fe9a65a5c9f
parent c6a0e55250831f5322760be73def174222a65943
Author: Masayuki Nakano <masayuki@d-toybox.com>
Date: Mon, 15 Dec 2025 07:50:40 +0000
Bug 2000978 - Make `SelectionMovementUtils::GetFrameForNodeOffset()` return `FrameAndOffset` instead of using `aReturnOffset` r=emilio,layout-reviewers
Additionally, this adds some shortcut operators to `FrameAndOffset`.
Therefore, some users of derived structs of `FrameAndOffset` become
simpler.
Differential Revision: https://phabricator.services.mozilla.com/D273716
Diffstat:
10 files changed, 149 insertions(+), 126 deletions(-)
diff --git a/dom/base/Selection.cpp b/dom/base/Selection.cpp
@@ -3713,7 +3713,6 @@ nsIFrame* Selection::GetSelectionEndPointGeometry(SelectionRegion aRegion,
nsINode* node = nullptr;
uint32_t nodeOffset = 0;
- nsIFrame* frame = nullptr;
switch (aRegion) {
case nsISelectionController::SELECTION_ANCHOR_REGION:
@@ -3732,12 +3731,14 @@ nsIFrame* Selection::GetSelectionEndPointGeometry(SelectionRegion aRegion,
nsCOMPtr<nsIContent> content = do_QueryInterface(node);
NS_ENSURE_TRUE(content.get(), nullptr);
- uint32_t frameOffset = 0;
- frame = SelectionMovementUtils::GetFrameForNodeOffset(
- content, nodeOffset, mFrameSelection->GetHint(), &frameOffset);
- if (!frame) return nullptr;
+ FrameAndOffset frameAndOffset = SelectionMovementUtils::GetFrameForNodeOffset(
+ content, nodeOffset, mFrameSelection->GetHint());
+ if (!frameAndOffset) {
+ return nullptr;
+ }
- SelectionMovementUtils::AdjustFrameForLineStart(frame, frameOffset);
+ SelectionMovementUtils::AdjustFrameForLineStart(
+ frameAndOffset.mFrame, frameAndOffset.mOffsetInFrameContent);
// Figure out what node type we have, then get the
// appropriate rect for its nodeOffset.
@@ -3747,16 +3748,21 @@ nsIFrame* Selection::GetSelectionEndPointGeometry(SelectionRegion aRegion,
if (isText) {
nsIFrame* childFrame = nullptr;
int32_t frameOffset = 0;
- nsresult rv = frame->GetChildFrameContainingOffset(
+ nsresult rv = frameAndOffset->GetChildFrameContainingOffset(
+ // FIXME: nodeOffset is offset in content (same as node) but
+ // frameAndOffset.mFrame may be a frame for its descendant. Therefore,
+ // frameAndOffset.mOffsetInFrameContent should be used here.
nodeOffset, mFrameSelection->GetHint() == CaretAssociationHint::After,
&frameOffset, &childFrame);
if (NS_FAILED(rv)) return nullptr;
if (!childFrame) return nullptr;
- frame = childFrame;
+ frameAndOffset.mFrame = childFrame;
// Get the coordinates of the offset into the text frame.
- rv = GetCachedFrameOffset(frame, nodeOffset, pt);
+ rv = GetCachedFrameOffset(
+ frameAndOffset.mFrame,
+ static_cast<int32_t>(frameAndOffset.mOffsetInFrameContent), pt);
if (NS_FAILED(rv)) return nullptr;
}
@@ -3767,7 +3773,7 @@ nsIFrame* Selection::GetSelectionEndPointGeometry(SelectionRegion aRegion,
// The block position and size are set so as to fill the frame in that axis.
// (i.e. block-position of 0, and block-size matching the frame's own block
// size).
- const WritingMode wm = frame->GetWritingMode();
+ const WritingMode wm = frameAndOffset->GetWritingMode();
// Helper to determine the inline-axis position for the aRect outparam.
auto GetInlinePosition = [&]() {
if (isText) {
@@ -3779,20 +3785,20 @@ nsIFrame* Selection::GetSelectionEndPointGeometry(SelectionRegion aRegion,
// inline-end edge (rather than physical end of inline axis)? (i.e. if we
// have direction:rtl, maybe this code would want to return 0 instead of
// height/width?)
- return frame->ISize(wm);
+ return frameAndOffset->ISize(wm);
};
// Set the inline position and block-size. Leave inline size and block
// position set to 0, as discussed above.
if (wm.IsVertical()) {
aRect->y = GetInlinePosition();
- aRect->SetWidth(frame->BSize(wm));
+ aRect->SetWidth(frameAndOffset->BSize(wm));
} else {
aRect->x = GetInlinePosition();
- aRect->SetHeight(frame->BSize(wm));
+ aRect->SetHeight(frameAndOffset->BSize(wm));
}
- return frame;
+ return frameAndOffset;
}
NS_IMETHODIMP
@@ -4229,7 +4235,7 @@ void Selection::Modify(const nsAString& aAlter, const nsAString& aDirection,
// we may have to swap the direction of movement.
const PrimaryFrameData frameForFocus =
GetPrimaryFrameForCaretAtFocusNode(visual);
- if (frameForFocus.mFrame) {
+ if (frameForFocus) {
if (visual) {
// FYI: This was done during a call of GetPrimaryFrameForCaretAtFocusNode.
// Therefore, this may not be intended by the original author.
diff --git a/dom/events/ContentEventHandler.cpp b/dom/events/ContentEventHandler.cpp
@@ -1129,18 +1129,18 @@ nsresult ContentEventHandler::ExpandToClusterBoundary(
MOZ_DIAGNOSTIC_ASSERT(mDocument->GetPresShell());
CaretAssociationHint hint =
aForward ? CaretAssociationHint::Before : CaretAssociationHint::After;
- nsIFrame* frame = SelectionMovementUtils::GetFrameForNodeOffset(
+ FrameAndOffset frameAndOffset = SelectionMovementUtils::GetFrameForNodeOffset(
&aTextNode, int32_t(*aXPOffset), hint);
- if (frame) {
- auto [startOffset, endOffset] = frame->GetOffsets();
+ if (frameAndOffset) {
+ auto [startOffset, endOffset] = frameAndOffset->GetOffsets();
if (*aXPOffset == static_cast<uint32_t>(startOffset) ||
*aXPOffset == static_cast<uint32_t>(endOffset)) {
return NS_OK;
}
- if (!frame->IsTextFrame()) {
+ if (!frameAndOffset->IsTextFrame()) {
return NS_ERROR_FAILURE;
}
- nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
+ nsTextFrame* textFrame = static_cast<nsTextFrame*>(frameAndOffset.mFrame);
int32_t newOffsetInFrame = *aXPOffset - startOffset;
newOffsetInFrame += aForward ? -1 : 1;
// PeekOffsetCharacter() should respect cluster but ignore user-select
diff --git a/layout/base/AccessibleCaretManager.cpp b/layout/base/AccessibleCaretManager.cpp
@@ -675,9 +675,11 @@ nsresult AccessibleCaretManager::SelectWordOrShortcut(const nsPoint& aPoint) {
if (offsets.content) {
RefPtr<nsFrameSelection> frameSelection = GetFrameSelection();
if (frameSelection) {
- nsIFrame* theFrame = SelectionMovementUtils::GetFrameForNodeOffset(
- offsets.content, offsets.offset, offsets.associate);
- if (theFrame && theFrame != ptFrame) {
+ const FrameAndOffset textFrameAndOffsetContainingWordBoundary =
+ SelectionMovementUtils::GetFrameForNodeOffset(
+ offsets.content, offsets.offset, offsets.associate);
+ if (textFrameAndOffsetContainingWordBoundary &&
+ textFrameAndOffsetContainingWordBoundary != ptFrame) {
SetSelectionDragState(true);
frameSelection->HandleClick(
MOZ_KnownLive(offsets.content) /* bug 1636889 */,
@@ -1221,7 +1223,7 @@ bool AccessibleCaretManager::RestrictCaretDraggingOffsets(
: GetLastVisibleLeafFrameOrUnselectableChildFrame(
*GetSelection()->GetLastRange(),
getter_AddRefs(content), &offsetInContent);
- if (!frameAndOffset.mFrame) {
+ if (!frameAndOffset) {
return false;
}
@@ -1248,7 +1250,7 @@ bool AccessibleCaretManager::RestrictCaretDraggingOffsets(
eSelectCluster, dir,
static_cast<int32_t>(frameAndOffset.mOffsetInFrameContent), nsPoint(0, 0),
{PeekOffsetOption::JumpLines, PeekOffsetOption::StopAtScroller});
- nsresult rv = frameAndOffset.mFrame->PeekOffset(&limit);
+ nsresult rv = frameAndOffset->PeekOffset(&limit);
if (NS_FAILED(rv)) {
limit.mResultContent = content;
limit.mContentOffset = offsetInContent;
diff --git a/layout/base/CaretAssociationHint.cpp b/layout/base/CaretAssociationHint.cpp
@@ -57,7 +57,7 @@ CaretAssociationHint ComputeCaretAssociationHint(
return CaretAssociationHint::After;
}
}
- return frameData.mFrame ? frameData.mHint : aDefault;
+ return frameData ? frameData.mHint : aDefault;
}
} // namespace mozilla
diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp
@@ -9304,7 +9304,7 @@ static bool LineHasNonEmptyContent(nsLineBox* aLine) {
}
/* static */
-bool nsLayoutUtils::IsInvisibleBreak(nsINode* aNode,
+bool nsLayoutUtils::IsInvisibleBreak(const nsINode* aNode,
nsIFrame** aNextLineFrame) {
if (aNextLineFrame) {
*aNextLineFrame = nullptr;
diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h
@@ -3056,7 +3056,7 @@ class nsLayoutUtils {
* assigned to first frame on the next line if such a next line exists, null
* otherwise.
*/
- static bool IsInvisibleBreak(nsINode* aNode,
+ static bool IsInvisibleBreak(const nsINode* aNode,
nsIFrame** aNextLineFrame = nullptr);
static nsRect ComputeSVGOriginBox(mozilla::dom::SVGViewportElement*);
diff --git a/layout/generic/SelectionMovementUtils.cpp b/layout/generic/SelectionMovementUtils.cpp
@@ -97,7 +97,7 @@ SelectionMovementUtils::PeekOffsetForCaretMove(
SelectionMovementUtils::GetPrimaryFrameForCaret(
aContent, aOffset, aOptions.contains(PeekOffsetOption::Visual), aHint,
aCaretBidiLevel);
- if (!frameForFocus.mFrame) {
+ if (!frameForFocus) {
return Err(NS_ERROR_FAILURE);
}
@@ -106,7 +106,7 @@ SelectionMovementUtils::PeekOffsetForCaretMove(
aAmount, aDirection,
static_cast<int32_t>(frameForFocus.mOffsetInFrameContent),
aDesiredCaretPos, aOptions, eDefaultBehavior, aAncestorLimiter);
- nsresult rv = frameForFocus.mFrame->PeekOffset(&pos);
+ nsresult rv = frameForFocus->PeekOffset(&pos);
if (NS_FAILED(rv)) {
return Err(rv);
}
@@ -118,33 +118,36 @@ nsPrevNextBidiLevels SelectionMovementUtils::GetPrevNextBidiLevels(
nsIContent* aNode, uint32_t aContentOffset, CaretAssociationHint aHint,
bool aJumpLines, const Element* aAncestorLimiter) {
// Get the level of the frames on each side
- nsIFrame* currentFrame;
- uint32_t currentOffset;
nsDirection direction;
nsPrevNextBidiLevels levels{};
levels.SetData(nullptr, nullptr, intl::BidiEmbeddingLevel::LTR(),
intl::BidiEmbeddingLevel::LTR());
- currentFrame = SelectionMovementUtils::GetFrameForNodeOffset(
- aNode, aContentOffset, aHint, ¤tOffset);
- if (!currentFrame) {
+ FrameAndOffset currentFrameAndOffset =
+ SelectionMovementUtils::GetFrameForNodeOffset(aNode, aContentOffset,
+ aHint);
+ if (!currentFrameAndOffset) {
return levels;
}
- auto [frameStart, frameEnd] = currentFrame->GetOffsets();
+ auto [frameStart, frameEnd] = currentFrameAndOffset->GetOffsets();
if (0 == frameStart && 0 == frameEnd) {
direction = eDirPrevious;
- } else if (static_cast<uint32_t>(frameStart) == currentOffset) {
+ } else if (static_cast<uint32_t>(frameStart) ==
+ currentFrameAndOffset.mOffsetInFrameContent) {
direction = eDirPrevious;
- } else if (static_cast<uint32_t>(frameEnd) == currentOffset) {
+ } else if (static_cast<uint32_t>(frameEnd) ==
+ currentFrameAndOffset.mOffsetInFrameContent) {
direction = eDirNext;
} else {
// we are neither at the beginning nor at the end of the frame, so we have
// no worries
- intl::BidiEmbeddingLevel currentLevel = currentFrame->GetEmbeddingLevel();
- levels.SetData(currentFrame, currentFrame, currentLevel, currentLevel);
+ intl::BidiEmbeddingLevel currentLevel =
+ currentFrameAndOffset->GetEmbeddingLevel();
+ levels.SetData(currentFrameAndOffset.mFrame, currentFrameAndOffset.mFrame,
+ currentLevel, currentLevel);
return levels;
}
@@ -152,12 +155,12 @@ nsPrevNextBidiLevels SelectionMovementUtils::GetPrevNextBidiLevels(
if (aJumpLines) {
peekOffsetOptions += PeekOffsetOption::JumpLines;
}
- nsIFrame* newFrame = currentFrame
+ nsIFrame* newFrame = currentFrameAndOffset
->GetFrameFromDirection(direction, peekOffsetOptions,
aAncestorLimiter)
.mFrame;
- FrameBidiData currentBidi = currentFrame->GetBidiData();
+ FrameBidiData currentBidi = currentFrameAndOffset->GetBidiData();
intl::BidiEmbeddingLevel currentLevel = currentBidi.embeddingLevel;
intl::BidiEmbeddingLevel newLevel =
newFrame ? newFrame->GetEmbeddingLevel() : currentBidi.baseLevel;
@@ -166,8 +169,8 @@ nsPrevNextBidiLevels SelectionMovementUtils::GetPrevNextBidiLevels(
// incorrectly.
// XXX This could be removed once bug 339786 is fixed.
if (!aJumpLines) {
- if (currentFrame->IsBrFrame()) {
- currentFrame = nullptr;
+ if (currentFrameAndOffset->IsBrFrame()) {
+ currentFrameAndOffset = {nullptr, 0u};
currentLevel = currentBidi.baseLevel;
}
if (newFrame && newFrame->IsBrFrame()) {
@@ -177,9 +180,11 @@ nsPrevNextBidiLevels SelectionMovementUtils::GetPrevNextBidiLevels(
}
if (direction == eDirNext) {
- levels.SetData(currentFrame, newFrame, currentLevel, newLevel);
+ levels.SetData(currentFrameAndOffset.mFrame, newFrame, currentLevel,
+ newLevel);
} else {
- levels.SetData(newFrame, currentFrame, newLevel, currentLevel);
+ levels.SetData(newFrame, currentFrameAndOffset.mFrame, newLevel,
+ currentLevel);
}
return levels;
@@ -246,23 +251,22 @@ static bool IsDisplayContents(const nsIContent* aContent) {
}
// static
-nsIFrame* SelectionMovementUtils::GetFrameForNodeOffset(
- nsIContent* aNode, uint32_t aOffset, CaretAssociationHint aHint,
- uint32_t* aReturnOffset /* = nullptr */) {
+FrameAndOffset SelectionMovementUtils::GetFrameForNodeOffset(
+ const nsIContent* aNode, uint32_t aOffset, CaretAssociationHint aHint) {
if (!aNode) {
- return nullptr;
+ return {};
}
if (static_cast<int32_t>(aOffset) < 0) {
- return nullptr;
+ return {};
}
if (!aNode->GetPrimaryFrame() && !IsDisplayContents(aNode)) {
- return nullptr;
+ return {};
}
nsIFrame *returnFrame = nullptr, *lastFrame = aNode->GetPrimaryFrame();
- nsCOMPtr<nsIContent> theNode;
+ const nsIContent* theNode = nullptr;
uint32_t offsetInFrameContent, offsetInLastFrameContent = aOffset;
while (true) {
@@ -316,11 +320,10 @@ nsIFrame* SelectionMovementUtils::GetFrameForNodeOffset(
aOffset = aOffset > childIndex ? theNode->GetChildCount() : 0;
continue;
}
+
// Check to see if theNode is a text node. If it is, translate
// aOffset into an offset into the text node.
-
- RefPtr<Text> textNode = theNode->GetAsText();
- if (textNode) {
+ if (const Text* textNode = Text::FromNode(theNode)) {
if (theNode->GetPrimaryFrame()) {
if (aOffset > childIndex) {
uint32_t textLength = textNode->Length();
@@ -339,7 +342,7 @@ nsIFrame* SelectionMovementUtils::GetFrameForNodeOffset(
nsCOMPtr<nsIContent> newChildNode =
aNode->GetChildAt_Deprecated(newChildIndex);
if (!newChildNode) {
- return nullptr;
+ return {};
}
aNode = newChildNode;
@@ -357,32 +360,36 @@ nsIFrame* SelectionMovementUtils::GetFrameForNodeOffset(
// If the node is a ShadowRoot, the frame needs to be adjusted,
// because a ShadowRoot does not get a frame. Its children are rendered
// as children of the host.
- if (ShadowRoot* shadow = ShadowRoot::FromNode(theNode)) {
+ if (const ShadowRoot* shadow = ShadowRoot::FromNode(theNode)) {
theNode = shadow->GetHost();
}
returnFrame = theNode->GetPrimaryFrame();
- if (!returnFrame) {
- if (aHint == CaretAssociationHint::Before) {
- if (aOffset > 0) {
- --aOffset;
- continue;
- }
- break;
- }
- if (aOffset < theNode->GetChildCount()) {
- ++aOffset;
+ if (returnFrame) {
+ // FIXME: offsetInFrameContent has not been updated for theNode yet when
+ // theNode is different from aNode. E.g., if a child at aNode and aOffset
+ // is an <img>, theNode is now the <img> but offsetInFrameContent is the
+ // offset for aNode.
+ break;
+ }
+
+ if (aHint == CaretAssociationHint::Before) {
+ if (aOffset > 0) {
+ --aOffset;
continue;
}
break;
}
-
+ if (aOffset < theNode->GetChildCount()) {
+ ++aOffset;
+ continue;
+ }
break;
} // end while
if (!returnFrame) {
if (!lastFrame) {
- return nullptr;
+ return {};
}
returnFrame = lastFrame;
offsetInFrameContent = offsetInLastFrameContent;
@@ -405,10 +412,7 @@ nsIFrame* SelectionMovementUtils::GetFrameForNodeOffset(
returnFrame->GetChildFrameContainingOffset(
static_cast<int32_t>(offsetInFrameContent),
aHint == CaretAssociationHint::After, &unused, &returnFrame);
- if (aReturnOffset) {
- *aReturnOffset = offsetInFrameContent;
- }
- return returnFrame;
+ return {returnFrame, offsetInFrameContent};
}
// static
@@ -755,12 +759,14 @@ CaretFrameData SelectionMovementUtils::GetCaretFrameForNodeOffset(
MOZ_ASSERT_IF(aForceEditableRegion == ForceEditableRegion::Yes,
aContentNode->IsEditable());
- result.mFrame = result.mUnadjustedFrame =
- SelectionMovementUtils::GetFrameForNodeOffset(
- aContentNode, aOffset, aFrameHint, &result.mOffsetInFrameContent);
- if (!result.mFrame) {
+ const FrameAndOffset frameAndOffset =
+ SelectionMovementUtils::GetFrameForNodeOffset(aContentNode, aOffset,
+ aFrameHint);
+ if (!frameAndOffset) {
return {};
}
+ result.mFrame = result.mUnadjustedFrame = frameAndOffset.mFrame;
+ result.mOffsetInFrameContent = frameAndOffset.mOffsetInFrameContent;
if (SelectionMovementUtils::AdjustFrameForLineStart(
result.mFrame, result.mOffsetInFrameContent)) {
@@ -779,14 +785,14 @@ CaretFrameData SelectionMovementUtils::GetCaretFrameForNodeOffset(
//
// Direction Style from visibility->mDirection
// ------------------
- if (!result.mFrame->PresContext()->BidiEnabled()) {
+ if (!result->PresContext()->BidiEnabled()) {
return result;
}
// If there has been a reflow, take the caret Bidi level to be the level of
// the current frame
if (aBidiLevel & BIDI_LEVEL_UNDEFINED) {
- aBidiLevel = result.mFrame->GetEmbeddingLevel();
+ aBidiLevel = result->GetEmbeddingLevel();
}
nsIFrame* frameBefore;
@@ -796,7 +802,7 @@ CaretFrameData SelectionMovementUtils::GetCaretFrameForNodeOffset(
intl::BidiEmbeddingLevel
levelAfter; // Bidi level of the character after the caret
- auto [start, end] = result.mFrame->GetOffsets();
+ auto [start, end] = result->GetOffsets();
if (start == 0 || end == 0 ||
static_cast<uint32_t>(start) == result.mOffsetInFrameContent ||
static_cast<uint32_t>(end) == result.mOffsetInFrameContent) {
@@ -829,7 +835,7 @@ CaretFrameData SelectionMovementUtils::GetCaretFrameForNodeOffset(
if (result.mFrame != frameBefore) {
if (frameBefore) { // if there is a frameBefore, move into it
result.mFrame = frameBefore;
- std::tie(start, end) = result.mFrame->GetOffsets();
+ std::tie(start, end) = result->GetOffsets();
result.mOffsetInFrameContent = end;
} else {
// if there is no frameBefore, we must be at the beginning of
@@ -860,7 +866,7 @@ CaretFrameData SelectionMovementUtils::GetCaretFrameForNodeOffset(
if (frameAfter) {
// if there is a frameAfter, move into it
result.mFrame = frameAfter;
- std::tie(start, end) = result.mFrame->GetOffsets();
+ std::tie(start, end) = result->GetOffsets();
result.mOffsetInFrameContent = start;
} else {
// if there is no frameAfter, we must be at the end of the line
@@ -895,8 +901,8 @@ CaretFrameData SelectionMovementUtils::GetCaretFrameForNodeOffset(
aBidiLevel);
if (MOZ_LIKELY(frameOrError.isOk())) {
result.mFrame = frameOrError.unwrap();
- std::tie(start, end) = result.mFrame->GetOffsets();
- levelAfter = result.mFrame->GetEmbeddingLevel();
+ std::tie(start, end) = result->GetOffsets();
+ levelAfter = result->GetEmbeddingLevel();
if (aBidiLevel.IsRTL()) {
// c8: caret to the right of the rightmost character
result.mOffsetInFrameContent = levelAfter.IsRTL() ? start : end;
@@ -919,8 +925,8 @@ CaretFrameData SelectionMovementUtils::GetCaretFrameForNodeOffset(
frameBefore, eDirPrevious, aBidiLevel);
if (MOZ_LIKELY(frameOrError.isOk())) {
result.mFrame = frameOrError.unwrap();
- std::tie(start, end) = result.mFrame->GetOffsets();
- levelBefore = result.mFrame->GetEmbeddingLevel();
+ std::tie(start, end) = result->GetOffsets();
+ levelBefore = result->GetEmbeddingLevel();
if (aBidiLevel.IsRTL()) {
// c12: caret to the left of the leftmost character
result.mOffsetInFrameContent = levelBefore.IsRTL() ? end : start;
@@ -947,7 +953,7 @@ PrimaryFrameData SelectionMovementUtils::GetPrimaryFrameForCaret(
const PrimaryFrameData result =
SelectionMovementUtils::GetPrimaryOrCaretFrameForNodeOffset(
aContent, aOffset, aVisual, aHint, aCaretBidiLevel);
- if (result.mFrame) {
+ if (result) {
return result;
}
}
@@ -981,13 +987,12 @@ PrimaryFrameData SelectionMovementUtils::GetPrimaryOrCaretFrameForNodeOffset(
nullptr, aContent, aOffset, aHint, aCaretBidiLevel,
aContent && aContent->IsEditable() ? ForceEditableRegion::Yes
: ForceEditableRegion::No);
- return {{result.mFrame, result.mOffsetInFrameContent}, result.mHint};
+ return result;
}
- uint32_t offset = 0;
- nsIFrame* theFrame = SelectionMovementUtils::GetFrameForNodeOffset(
- aContent, aOffset, aHint, &offset);
- return {{theFrame, offset}, aHint};
+ return {
+ SelectionMovementUtils::GetFrameForNodeOffset(aContent, aOffset, aHint),
+ aHint};
}
} // namespace mozilla
diff --git a/layout/generic/SelectionMovementUtils.h b/layout/generic/SelectionMovementUtils.h
@@ -30,6 +30,16 @@ struct MOZ_STACK_CLASS FrameAndOffset {
return mFrame ? mFrame->GetContent() : nullptr;
}
+ operator nsIFrame*() const { return mFrame; }
+
+ explicit operator bool() const { return !!mFrame; }
+ [[nodiscard]] bool operator!() const { return !mFrame; }
+
+ nsIFrame* operator->() const {
+ MOZ_ASSERT(mFrame);
+ return mFrame;
+ }
+
nsIFrame* mFrame = nullptr;
// The offset in mFrame->GetContent().
uint32_t mOffsetInFrameContent = 0;
@@ -87,13 +97,11 @@ class SelectionMovementUtils final {
* that frame.
*
* @param aNode input parameter for the node to look at
- * TODO: Make this `const nsIContent*` for `ContentEventHandler`.
* @param aOffset offset into above node.
- * @param aReturnOffset will contain offset into frame.
*/
- static nsIFrame* GetFrameForNodeOffset(nsIContent* aNode, uint32_t aOffset,
- CaretAssociationHint aHint,
- uint32_t* aReturnOffset = nullptr);
+ static FrameAndOffset GetFrameForNodeOffset(const nsIContent* aNode,
+ uint32_t aOffset,
+ CaretAssociationHint aHint);
/**
* Return the first visible point in or at a leaf node in aRange or the first
diff --git a/layout/generic/nsFrameSelection.cpp b/layout/generic/nsFrameSelection.cpp
@@ -803,7 +803,7 @@ nsresult nsFrameSelection::MoveCaret(nsDirection aDirection,
Caret::IsVisualMovement(aExtendSelection, aMovementStyle);
const PrimaryFrameData frameForFocus =
sel->GetPrimaryFrameForCaretAtFocusNode(visualMovement);
- if (!frameForFocus.mFrame) {
+ if (!frameForFocus) {
return NS_ERROR_FAILURE;
}
if (visualMovement) {
@@ -1104,8 +1104,7 @@ void nsFrameSelection::BidiLevelFromMove(PresShell* aPresShell,
void nsFrameSelection::BidiLevelFromClick(nsIContent* aNode,
uint32_t aContentOffset) {
- nsIFrame* clickInFrame = nullptr;
- clickInFrame = SelectionMovementUtils::GetFrameForNodeOffset(
+ nsIFrame* clickInFrame = SelectionMovementUtils::GetFrameForNodeOffset(
aNode, aContentOffset, mCaret.mHint);
if (!clickInFrame) {
return;
@@ -1184,30 +1183,33 @@ void nsFrameSelection::MaintainedRange::AdjustContentOffsets(
amount = eSelectEndLine;
}
- uint32_t offset;
- nsIFrame* frame = SelectionMovementUtils::GetFrameForNodeOffset(
- aOffsets.content, aOffsets.offset, CaretAssociationHint::After,
- &offset);
+ FrameAndOffset frameAndOffset =
+ SelectionMovementUtils::GetFrameForNodeOffset(
+ aOffsets.content, aOffsets.offset, CaretAssociationHint::After);
PeekOffsetOptions peekOffsetOptions{};
if (aStopAtScroller == StopAtScroller::Yes) {
peekOffsetOptions += PeekOffsetOption::StopAtScroller;
}
- if (frame && amount == eSelectWord && direction == eDirPrevious) {
+ if (frameAndOffset && amount == eSelectWord && direction == eDirPrevious) {
// To avoid selecting the previous word when at start of word,
// first move one character forward.
- PeekOffsetStruct charPos(eSelectCharacter, eDirNext,
- static_cast<int32_t>(offset), nsPoint(0, 0),
- peekOffsetOptions);
- if (NS_SUCCEEDED(frame->PeekOffset(&charPos))) {
- frame = charPos.mResultFrame;
- offset = charPos.mContentOffset;
+ PeekOffsetStruct charPos(
+ eSelectCharacter, eDirNext,
+ static_cast<int32_t>(frameAndOffset.mOffsetInFrameContent),
+ nsPoint(0, 0), peekOffsetOptions);
+ if (NS_SUCCEEDED(frameAndOffset->PeekOffset(&charPos))) {
+ frameAndOffset = {charPos.mResultFrame,
+ static_cast<uint32_t>(charPos.mContentOffset)};
}
}
- PeekOffsetStruct pos(amount, direction, static_cast<int32_t>(offset),
- nsPoint(0, 0), peekOffsetOptions);
- if (frame && NS_SUCCEEDED(frame->PeekOffset(&pos)) && pos.mResultContent) {
+ PeekOffsetStruct pos(
+ amount, direction,
+ static_cast<int32_t>(frameAndOffset.mOffsetInFrameContent),
+ nsPoint(0, 0), peekOffsetOptions);
+ if (frameAndOffset && NS_SUCCEEDED(frameAndOffset->PeekOffset(&pos)) &&
+ pos.mResultContent) {
aOffsets.content = pos.mResultContent;
aOffsets.offset = pos.mContentOffset;
}
@@ -1982,20 +1984,20 @@ nsresult nsFrameSelection::PhysicalMove(int16_t aDirection, int16_t aAmount,
WritingMode wm;
const PrimaryFrameData frameForFocus =
sel->GetPrimaryFrameForCaretAtFocusNode(true);
- if (frameForFocus.mFrame) {
+ if (frameForFocus) {
// FYI: Setting the caret association hint was done during a call of
// GetPrimaryFrameForCaretAtFocusNode. Therefore, this may not be intended
// by the original author.
sel->GetFrameSelection()->SetHint(frameForFocus.mHint);
- if (!frameForFocus.mFrame->Style()->IsTextCombined()) {
- wm = frameForFocus.mFrame->GetWritingMode();
+ if (!frameForFocus->Style()->IsTextCombined()) {
+ wm = frameForFocus->GetWritingMode();
} else {
// Using different direction for horizontal-in-vertical would
// make it hard to navigate via keyboard. Inherit the moving
// direction from its parent.
- MOZ_ASSERT(frameForFocus.mFrame->IsTextFrame());
- wm = frameForFocus.mFrame->GetParent()->GetWritingMode();
+ MOZ_ASSERT(frameForFocus->IsTextFrame());
+ wm = frameForFocus->GetParent()->GetWritingMode();
MOZ_ASSERT(wm.IsVertical(),
"Text combined "
"can only appear in vertical text");
diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp
@@ -5338,14 +5338,14 @@ nsresult nsIFrame::SelectByTypeAtPoint(const nsPoint& aPoint,
return NS_ERROR_FAILURE;
}
- uint32_t offset;
- nsIFrame* frame = SelectionMovementUtils::GetFrameForNodeOffset(
- offsets.content, offsets.offset, offsets.associate, &offset);
- if (!frame) {
+ FrameAndOffset frameAndOffset = SelectionMovementUtils::GetFrameForNodeOffset(
+ offsets.content, offsets.offset, offsets.associate);
+ if (!frameAndOffset) {
return NS_ERROR_FAILURE;
}
- return frame->PeekBackwardAndForwardForSelection(
- aBeginAmountType, aEndAmountType, static_cast<int32_t>(offset),
+ return frameAndOffset->PeekBackwardAndForwardForSelection(
+ aBeginAmountType, aEndAmountType,
+ static_cast<int32_t>(frameAndOffset.mOffsetInFrameContent),
aBeginAmountType != eSelectWord, aSelectFlags);
}
