Bug 1302470 Part 6: Properly check for a frame's visibility, do not abuse isRangeVisible() for that purpose. r=mikedeboer
authorMike de Boer <mdeboer@mozilla.com>
Fri, 28 Apr 2017 19:06:35 +0200
changeset 427884 db3f44db75c5dbd74c013bc9467587f6d1bec050
parent 427883 20c8d9fcd818e96bb42d1eaf672b7ddce1c21936
child 427885 5909ec7d5d53b840746b267c8668724bf8704906
push id97
push userfmarier@mozilla.com
push dateSat, 14 Oct 2017 01:12:59 +0000
reviewersmikedeboer
bugs1302470
milestone58.0a1
Bug 1302470 Part 6: Properly check for a frame's visibility, do not abuse isRangeVisible() for that purpose. r=mikedeboer MozReview-Commit-ID: ErviFQrJR1u
toolkit/modules/FinderIterator.jsm
--- a/toolkit/modules/FinderIterator.jsm
+++ b/toolkit/modules/FinderIterator.jsm
@@ -8,16 +8,17 @@ this.EXPORTED_SYMBOLS = ["FinderIterator
 
 const { interfaces: Ci, classes: Cc, utils: Cu } = Components;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "NLP", "resource://gre/modules/NLP.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Rect", "resource://gre/modules/Geometry.jsm");
 
 const kDebug = false;
 const kIterationSizeMax = 100;
 const kTimeoutPref = "findbar.iteratorTimeout";
 
 /**
  * FinderIterator singleton. See the documentation for the `start()` method to
  * learn more.
@@ -574,27 +575,23 @@ this.FinderIterator = {
    */
   _collectFrames(window, finder) {
     let frames = [];
     if (!("frames" in window) || !window.frames.length)
       return frames;
 
     // Casting `window.frames` to an Iterator doesn't work, so we're stuck with
     // a plain, old for-loop.
+    let dwu = window.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
     for (let i = 0, l = window.frames.length; i < l; ++i) {
       let frame = window.frames[i];
-      // Don't count matches in hidden frames.
+      // Don't count matches in hidden frames; get the frame element rect and
+      // check if it's empty. We shan't flush!
       let frameEl = frame && frame.frameElement;
-      if (!frameEl)
-        continue;
-      // Construct a range around the frame element to check its visiblity.
-      let range = window.document.createRange();
-      range.setStart(frameEl, 0);
-      range.setEnd(frameEl, 0);
-      if (!finder._fastFind.isRangeVisible(range, false))
+      if (!frameEl || Rect.fromRect(dwu.getBoundsWithoutFlushing(frameEl)).isEmpty())
         continue;
       // All conditions pass, so push the current frame and its children on the
       // stack.
       frames.push(frame, ...this._collectFrames(frame, finder));
     }
 
     return frames;
   },