Bug 1436431 Part 2: Change nsTypeAheadFind::IsRangeVisible to additionally check for visibility of range rects; not just the range's primary frame.
MozReview-Commit-ID: EQFyRswqy5L
Since a range rect can be considerably smaller than the rect of its
containing frame, this change avoids an unpleasant false-negative for
find-in-page. Without this change, if a frame is reported as visible,
but none of the range rects are visible, the later call to isRangeRendered
will report the text as not findable, even though it could be scrolled
into view. With this change, the text in such a case is reported as
findable, but just out of view, and the find-in-page logic will scroll
it into view as needed.
--- a/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
+++ b/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
@@ -1263,23 +1263,56 @@ nsTypeAheadFind::IsRangeVisible(nsIPresS
// We don't use the more accurate AccGetBounds, because that is
// more expensive and the STATE_OFFSCREEN flag that this is used
// for only needs to be a rough indicator
nsRectVisibility rectVisibility = nsRectVisibility_kAboveViewport;
if (!aGetTopVisibleLeaf && !frame->GetRect().IsEmpty()) {
rectVisibility =
aPresShell->GetRectVisibility(frame,
- nsRect(nsPoint(0,0), frame->GetSize()),
+ frame->GetRectRelativeToSelf(),
minDistance);
if (rectVisibility == nsRectVisibility_kVisible) {
- // This is an early exit case, where we return true if and only if
- // the range is actually rendered.
- return IsRangeRendered(aPresShell, aPresContext, aRange);
+ // The primary frame of the range is visible, but we don't yet know if
+ // any of the rects of the range itself are visible. Check to see if at
+ // least one of the rects is visible.
+ bool atLeastOneRangeRectVisible = false;
+
+ nsIFrame* containerFrame =
+ nsLayoutUtils::GetContainingBlockForClientRect(frame);
+ RefPtr<DOMRectList> rects = aRange->GetClientRects(true, true);
+ for (uint32_t i = 0; i < rects->Length(); ++i) {
+ RefPtr<DOMRect> rect = rects->Item(i);
+ nsRect r(nsPresContext::CSSPixelsToAppUnits((float)rect->X()),
+ nsPresContext::CSSPixelsToAppUnits((float)rect->Y()),
+ nsPresContext::CSSPixelsToAppUnits((float)rect->Width()),
+ nsPresContext::CSSPixelsToAppUnits((float)rect->Height()));
+
+ // r is relative to containerFrame; transform it back to frame, so we
+ // can do a proper visibility check that is cropped to all of frame's
+ // ancestor scroll frames.
+ nsLayoutUtils::TransformResult res =
+ nsLayoutUtils::TransformRect(containerFrame, frame, r);
+ if (res == nsLayoutUtils::TransformResult::TRANSFORM_SUCCEEDED) {
+ nsRectVisibility rangeRectVisibility =
+ aPresShell->GetRectVisibility(frame, r, minDistance);
+
+ if (rangeRectVisibility == nsRectVisibility_kVisible) {
+ atLeastOneRangeRectVisible = true;
+ break;
+ }
+ }
+ }
+
+ if (atLeastOneRangeRectVisible) {
+ // This is an early exit case, where we return true if and only if
+ // the range is actually rendered.
+ return IsRangeRendered(aPresShell, aPresContext, aRange);
+ }
}
}
// Below this point, we know the range is not in the viewport.
if (!aMustBeInViewPort) {
// This is an early exit case because we don't care that that range
// is out of viewport, so we return that the range is "visible".