[tbb-commits] [tor-browser] 206/311: Bug 1759866 - Minor clean-up to the selection frame-edge code. r=dholbert a=dmeehan
gitolite role
git at cupani.torproject.org
Tue Apr 26 15:30:06 UTC 2022
This is an automated email from the git hooks/post-receive script.
pierov pushed a commit to branch geckoview-99.0.1-11.0-1
in repository tor-browser.
commit 2a4b303d8387dd630c6cb91e3d6cdf993f377f73
Author: Emilio Cobos Álvarez <emilio at crisal.io>
AuthorDate: Wed Mar 16 18:55:01 2022 +0000
Bug 1759866 - Minor clean-up to the selection frame-edge code. r=dholbert a=dmeehan
This doesn't change behavior but makes some code a bit nicer to read and
documents some of the code that wasn't obvious otherwise.
Differential Revision: https://phabricator.services.mozilla.com/D141236
---
layout/generic/nsIFrame.cpp | 113 ++++++++++++++++++++++++--------------------
1 file changed, 63 insertions(+), 50 deletions(-)
diff --git a/layout/generic/nsIFrame.cpp b/layout/generic/nsIFrame.cpp
index a71d2ade036eb..c4fc8ac39f81f 100644
--- a/layout/generic/nsIFrame.cpp
+++ b/layout/generic/nsIFrame.cpp
@@ -4747,17 +4747,23 @@ nsresult nsIFrame::MoveCaretToEventPoint(nsPresContext* aPresContext,
return NS_OK;
}
- if (!aMouseEvent->IsAlt()) {
+ const nsPoint pt = nsLayoutUtils::GetEventCoordinatesRelativeTo(
+ aMouseEvent, RelativeTo{this});
+
+ // When not using `alt`, and clicking on a draggable, but non-editable
+ // element, don't do anything, and let d&d handle the event.
+ //
+ // See bug 48876, bug 388659 and bug 55921 for context here.
+ //
+ // FIXME(emilio): The .Contains(pt) check looks a bit fishy. When would it be
+ // false given we're the event target? If it is needed, why not checking the
+ // actual draggable node rect instead?
+ if (!aMouseEvent->IsAlt() && GetRectRelativeToSelf().Contains(pt)) {
for (nsIContent* content = mContent; content;
content = content->GetFlattenedTreeParent()) {
if (nsContentUtils::ContentIsDraggable(content) &&
!content->IsEditable()) {
- // coordinate stuff is the fix for bug #55921
- if ((mRect - GetPosition())
- .Contains(nsLayoutUtils::GetEventCoordinatesRelativeTo(
- aMouseEvent, RelativeTo{this}))) {
- return NS_OK;
- }
+ return NS_OK;
}
}
}
@@ -4816,9 +4822,9 @@ nsresult nsIFrame::MoveCaretToEventPoint(nsPresContext* aPresContext,
if (aMouseEvent->IsControl()) {
return NS_OK;
}
- bool control = aMouseEvent->IsMeta();
+ const bool control = aMouseEvent->IsMeta();
#else
- bool control = aMouseEvent->IsControl();
+ const bool control = aMouseEvent->IsControl();
#endif
RefPtr<nsFrameSelection> fc = const_cast<nsFrameSelection*>(frameselection);
@@ -4830,8 +4836,6 @@ nsresult nsIFrame::MoveCaretToEventPoint(nsPresContext* aPresContext,
control);
}
- nsPoint pt = nsLayoutUtils::GetEventCoordinatesRelativeTo(aMouseEvent,
- RelativeTo{this});
ContentOffsets offsets = GetContentOffsetsFromPoint(pt, SKIP_HIDDEN);
if (!offsets.content) {
@@ -4958,10 +4962,10 @@ nsresult nsIFrame::MoveCaretToEventPoint(nsPresContext* aPresContext,
if (isPrimaryButtonDown && isEditor && !aMouseEvent->IsShift() &&
(offsets.EndOffset() - offsets.StartOffset()) == 1) {
- // A single node is selected and we aren't extending an existing
- // selection, which means the user clicked directly on an object (either
- // user-select: all or a non-text node without children).
- // Therefore, disable selection extension during mouse moves.
+ // A single node is selected and we aren't extending an existing selection,
+ // which means the user clicked directly on an object (either
+ // `user-select: all` or a non-text node without children). Therefore,
+ // disable selection extension during mouse moves.
// XXX This is a bit hacky; shouldn't editor be able to deal with this?
fc->SetDragState(false);
}
@@ -5403,17 +5407,13 @@ static FrameContentRange GetRangeForFrame(const nsIFrame* aFrame) {
// The FrameTarget represents the closest frame to a point that can be selected
// The frame is the frame represented, frameEdge says whether one end of the
// frame is the result (in which case different handling is needed), and
-// afterFrame says which end is repersented if frameEdge is true
+// afterFrame says which end is represented if frameEdge is true
struct FrameTarget {
- FrameTarget(nsIFrame* aFrame, bool aFrameEdge, bool aAfterFrame)
- : frame(aFrame), frameEdge(aFrameEdge), afterFrame(aAfterFrame) {}
+ explicit operator bool() const { return !!frame; }
- static FrameTarget Null() { return FrameTarget(nullptr, false, false); }
-
- bool IsNull() { return !frame; }
- nsIFrame* frame;
- bool frameEdge;
- bool afterFrame;
+ nsIFrame* frame = nullptr;
+ bool frameEdge = false;
+ bool afterFrame = false;
};
// See function implementation for information
@@ -5467,7 +5467,7 @@ static FrameTarget GetSelectionClosestFrameForChild(nsIFrame* aChild,
nsPoint pt = aPoint - aChild->GetOffsetTo(parent);
return GetSelectionClosestFrame(aChild, pt, aFlags);
}
- return FrameTarget(aChild, false, false);
+ return FrameTarget{aChild, false, false};
}
// When the cursor needs to be at the beginning of a block, it shouldn't be
@@ -5498,7 +5498,7 @@ static FrameTarget DrillDownToSelectionFrame(nsIFrame* aFrame, bool aEndFrame,
if (result) return DrillDownToSelectionFrame(result, aEndFrame, aFlags);
}
// If the current frame has no targetable children, target the current frame
- return FrameTarget(aFrame, true, aEndFrame);
+ return FrameTarget{aFrame, true, aEndFrame};
}
// This method finds the closest valid FrameTarget on a given line; if there is
@@ -5552,7 +5552,7 @@ static FrameTarget GetSelectionClosestFrameForLine(
if (!closestFromIStart && !closestFromIEnd) {
// We should only get here if there are no selectable frames on a line
// XXX Do we need more elaborate handling here?
- return FrameTarget::Null();
+ return FrameTarget();
}
if (closestFromIStart &&
(!closestFromIEnd ||
@@ -5571,7 +5571,9 @@ static FrameTarget GetSelectionClosestFrameForBlock(nsIFrame* aFrame,
const nsPoint& aPoint,
uint32_t aFlags) {
nsBlockFrame* bf = do_QueryFrame(aFrame);
- if (!bf) return FrameTarget::Null();
+ if (!bf) {
+ return FrameTarget();
+ }
// This code searches for the correct line
nsBlockFrame::LineIterator end = bf->LinesEnd();
@@ -5625,9 +5627,10 @@ static FrameTarget GetSelectionClosestFrameForBlock(nsIFrame* aFrame,
}
do {
- FrameTarget target =
- GetSelectionClosestFrameForLine(bf, closestLine, aPoint, aFlags);
- if (!target.IsNull()) return target;
+ if (auto target =
+ GetSelectionClosestFrameForLine(bf, closestLine, aPoint, aFlags)) {
+ return target;
+ }
++closestLine;
} while (closestLine != end);
@@ -5635,6 +5638,23 @@ static FrameTarget GetSelectionClosestFrameForBlock(nsIFrame* aFrame,
return DrillDownToSelectionFrame(aFrame, true, aFlags);
}
+// Use frame edge for grid, flex, table, and non-draggable & non-editable
+// image frames.
+static bool UseFrameEdge(nsIFrame* aFrame) {
+ if (aFrame->IsFlexOrGridContainer() || aFrame->IsTableFrame()) {
+ return true;
+ }
+ if (static_cast<nsImageFrame*>(do_QueryFrame(aFrame))) {
+ return !nsContentUtils::ContentIsDraggable(aFrame->GetContent()) &&
+ !aFrame->GetContent()->IsEditable();
+ }
+ return false;
+}
+
+static FrameTarget LastResortFrameTargetForFrame(nsIFrame* aFrame) {
+ return {aFrame, UseFrameEdge(aFrame), false};
+}
+
// GetSelectionClosestFrame is the helper function that calculates the closest
// frame to the given point.
// It doesn't completely account for offset styles, so needs to be used in
@@ -5645,11 +5665,9 @@ static FrameTarget GetSelectionClosestFrameForBlock(nsIFrame* aFrame,
static FrameTarget GetSelectionClosestFrame(nsIFrame* aFrame,
const nsPoint& aPoint,
uint32_t aFlags) {
- {
- // Handle blocks; if the frame isn't a block, the method fails
- FrameTarget target =
- GetSelectionClosestFrameForBlock(aFrame, aPoint, aFlags);
- if (!target.IsNull()) return target;
+ // Handle blocks; if the frame isn't a block, the method fails
+ if (auto target = GetSelectionClosestFrameForBlock(aFrame, aPoint, aFlags)) {
+ return target;
}
if (nsIFrame* kid = aFrame->PrincipalChildList().FirstChild()) {
@@ -5662,19 +5680,12 @@ static FrameTarget GetSelectionClosestFrame(nsIFrame* aFrame,
}
if (closest.mFrame) {
if (SVGUtils::IsInSVGTextSubtree(closest.mFrame))
- return FrameTarget(closest.mFrame, false, false);
+ return FrameTarget{closest.mFrame, false, false};
return GetSelectionClosestFrameForChild(closest.mFrame, aPoint, aFlags);
}
}
- // Use frame edge for grid, flex, table, and non-draggable & non-editable
- // image frames.
- const bool useFrameEdge =
- aFrame->IsFlexOrGridContainer() || aFrame->IsTableFrame() ||
- (static_cast<nsImageFrame*>(do_QueryFrame(aFrame)) &&
- !nsContentUtils::ContentIsDraggable(aFrame->GetContent()) &&
- !aFrame->GetContent()->IsEditable());
- return FrameTarget(aFrame, useFrameEdge, false);
+ return LastResortFrameTargetForFrame(aFrame);
}
static nsIFrame::ContentOffsets OffsetsForSingleFrame(nsIFrame* aFrame,
@@ -5748,19 +5759,21 @@ nsIFrame::ContentOffsets nsIFrame::GetContentOffsetsFromPoint(
adjustedFrame = AdjustFrameForSelectionStyles(this);
- // user-select: all needs special handling, because clicking on it
- // should lead to the whole frame being selected
+ // `user-select: all` needs special handling, because clicking on it should
+ // lead to the whole frame being selected.
if (adjustedFrame->Style()->UserSelect() == StyleUserSelect::All) {
- nsPoint adjustedPoint = aPoint + this->GetOffsetTo(adjustedFrame);
+ nsPoint adjustedPoint = aPoint + GetOffsetTo(adjustedFrame);
return OffsetsForSingleFrame(adjustedFrame, adjustedPoint);
}
// For other cases, try to find a closest frame starting from the parent of
// the unselectable frame
- if (adjustedFrame != this) adjustedFrame = adjustedFrame->GetParent();
+ if (adjustedFrame != this) {
+ adjustedFrame = adjustedFrame->GetParent();
+ }
}
- nsPoint adjustedPoint = aPoint + this->GetOffsetTo(adjustedFrame);
+ nsPoint adjustedPoint = aPoint + GetOffsetTo(adjustedFrame);
FrameTarget closest =
GetSelectionClosestFrame(adjustedFrame, adjustedPoint, aFlags);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the tbb-commits
mailing list