Bug 1436431 Part 2: Change nsTypeAheadFind::IsRangeVisible to additionally check for visibility of range rects; not just the range's primary frame. r=bz
authorBrad Werth <bwerth@mozilla.com>
Wed, 25 Apr 2018 12:28:57 -0700
changeset 473049 ec1b3f950848608328c18c0102c55305538abe36
parent 473048 b0c260eb196cb1e214f8d5b0810e0ea93ebb7196
child 473050 dc092a92c289664b31977c4f5f1b986ae6a9f931
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1436431
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1436431 Part 2: Change nsTypeAheadFind::IsRangeVisible to additionally check for visibility of range rects; not just the range's primary frame. r=bz 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.
toolkit/components/typeaheadfind/nsTypeAheadFind.cpp
--- 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".