Bug 1280149 - rework and fix the positioning for range rects to work better with iframes and try better to not draw rects that are not visible from the top document. This basically means to leverage the Geometry.jsm API more and better. r=jaws
authorMike de Boer <mdeboer@mozilla.com>
Wed, 31 Aug 2016 09:59:56 +0200
changeset 311980 cb40070e2bf693e97d2b55af001a847f69c791bd
parent 311979 a3a061ad49c3394d2843e4120f994d9ecd45097d
child 311981 e81611b17f1af56aa898c6fd4908363f86161366
push id31872
push usermdeboer@mozilla.com
push dateWed, 31 Aug 2016 08:05:46 +0000
treeherderautoland@cb40070e2bf6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1280149
milestone51.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 1280149 - rework and fix the positioning for range rects to work better with iframes and try better to not draw rects that are not visible from the top document. This basically means to leverage the Geometry.jsm API more and better. r=jaws MozReview-Commit-ID: Kdr5FWzhxM4
toolkit/modules/FinderHighlighter.jsm
--- a/toolkit/modules/FinderHighlighter.jsm
+++ b/toolkit/modules/FinderHighlighter.jsm
@@ -590,41 +590,46 @@ FinderHighlighter.prototype = {
   },
 
   /**
    * Utility; returns the bounds of the page relative to the viewport.
    * If the pages is part of a frameset or inside an iframe of any kind, its
    * offset is accounted for.
    * Geometry.jsm takes care of the DOMRect calculations.
    *
-   * @param  {nsIDOMWindow} window
+   * @param  {nsIDOMWindow} window          Window to read the boundary rect from
+   * @param  {Boolean}      [includeScroll] Whether to ignore the scroll offset,
+   *                                        which is useful for comparing DOMRects.
+   *                                        Optional, defaults to `true`
    * @return {Rect}
    */
-  _getRootBounds(window) {
+  _getRootBounds(window, includeScroll = true) {
     let dwu = this._getDWU(window);
     let cssPageRect = Rect.fromRect(dwu.getRootBounds());
-
     let scrollX = {};
     let scrollY = {};
-    dwu.getScrollXY(false, scrollX, scrollY);
-    cssPageRect.translate(scrollX.value, scrollY.value);
+    if (includeScroll) {
+      dwu.getScrollXY(false, scrollX, scrollY);
+      cssPageRect.translate(scrollX.value, scrollY.value);
+    }
 
     // If we're in a frame, update the position of the rect (top/ left).
     let currWin = window;
     while (currWin != window.top) {
       // Since the frame is an element inside a parent window, we'd like to
       // learn its position relative to it.
       let el = this._getDWU(currWin).containerElement;
       currWin = window.parent;
       dwu = this._getDWU(currWin);
       let parentRect = Rect.fromRect(dwu.getBoundsWithoutFlushing(el));
 
-      // Always take the scroll position into account.
-      dwu.getScrollXY(false, scrollX, scrollY);
-      parentRect.translate(scrollX.value, scrollY.value);
+      if (includeScroll) {
+        dwu.getScrollXY(false, scrollX, scrollY);
+        parentRect.translate(scrollX.value, scrollY.value);
+      }
 
       cssPageRect.translate(parentRect.left, parentRect.top);
     }
 
     return cssPageRect;
   },
 
   /**
@@ -779,27 +784,28 @@ FinderHighlighter.prototype = {
       bounds = dict.frames.get(window);
       if (!bounds) {
         bounds = this._getRootBounds(window);
         dict.frames.set(window, bounds);
       }
     } else
       bounds = this._getRootBounds(window);
 
+    let topBounds = this._getRootBounds(window.top, false);
     let rects = new Set();
     // A range may consist of multiple rectangles, we can also do these kind of
     // precise cut-outs. range.getBoundingClientRect() returns the fully
     // encompassing rectangle, which is too much for our purpose here.
-    for (let dims of range.getClientRects()) {
-      rects.add({
-        height: dims.bottom - dims.top,
-        width: dims.right - dims.left,
-        y: dims.top + bounds.top,
-        x: dims.left + bounds.left
-      });
+    for (let rect of range.getClientRects()) {
+      rect = Rect.fromRect(rect);
+      rect.x += bounds.x;
+      rect.y += bounds.y;
+      // If the rect is not even visible from the top document, we can ignore it.
+      if (rect.intersects(topBounds))
+        rects.add(rect);
     }
 
     dict = dict || this.getForWindow(window.top);
     dict.modalHighlightRectsMap.set(range, rects);
     if (checkIfDynamic && this._isInDynamicContainer(range))
       dict.dynamicRangesSet.add(range);
     return rects;
   },
@@ -889,16 +895,18 @@ FinderHighlighter.prototype = {
   _maybeCreateModalHighlightNodes(window) {
     window = window.top;
     let dict = this.getForWindow(window);
     if (dict.modalHighlightOutline) {
       if (!dict.modalHighlightAllMask) {
         // Make sure to at least show the dimmed background.
         this._repaintHighlightAllMask(window, false);
         this._scheduleRepaintOfMask(window);
+      } else {
+        this._scheduleRepaintOfMask(window, { scrollOnly: true });
       }
       return;
     }
 
     let document = window.document;
     // A hidden document doesn't accept insertAnonymousContent calls yet.
     if (document.hidden) {
       let onVisibilityChange = () => {
@@ -1044,32 +1052,30 @@ FinderHighlighter.prototype = {
       dict.updateAllRanges = updateAllRanges;
 
     if (dict.modalRepaintScheduler)
       return;
 
     dict.modalRepaintScheduler = window.setTimeout(() => {
       dict.modalRepaintScheduler = null;
 
-      if (dict.unconditionalRepaintRequested) {
+      let { width: previousWidth, height: previousHeight } = dict.lastWindowDimensions;
+      let { width, height } = dict.lastWindowDimensions = this._getWindowDimensions(window);
+      let pageContentChanged = (Math.abs(previousWidth - width) > kContentChangeThresholdPx ||
+                                Math.abs(previousHeight - height) > kContentChangeThresholdPx);
+      // When the page has changed significantly enough in size, we'll restart
+      // the iterator with the same parameters as before to find us new ranges.
+      if (pageContentChanged)
+        this.iterator.restart(this.finder);
+
+      if (dict.unconditionalRepaintRequested ||
+          (dict.modalHighlightRectsMap.size && pageContentChanged)) {
         dict.unconditionalRepaintRequested = false;
         this._repaintHighlightAllMask(window);
-        return;
       }
-
-      let { width, height } = this._getWindowDimensions(window);
-      if (!dict.modalHighlightRectsMap.size ||
-          (Math.abs(dict.lastWindowDimensions.width - width) < kContentChangeThresholdPx &&
-           Math.abs(dict.lastWindowDimensions.height - height) < kContentChangeThresholdPx)) {
-        return;
-      }
-
-      this.iterator.restart(this.finder);
-      dict.lastWindowDimensions = { width, height };
-      this._repaintHighlightAllMask(window);
     }, kModalHighlightRepaintFreqMs);
   },
 
   /**
    * The outline that shows/ highlights the current found range is styled and
    * animated using CSS. This style can be found in `kModalStyle`, but to have it
    * applied on any DOM node we insert using the AnonymousContent API we need to
    * inject an agent sheet into the document.