Bug 1279652 - reddit.com invalidates all references in the TextLayer, resulting in zero ClientRects for ranges that we have in the FinderHighlighter cache. This fix makes sure to re-fetch all the ranges when this is detected. r?jaws draft
authorMike de Boer <mdeboer@mozilla.com>
Tue, 11 Oct 2016 11:18:14 +0200
changeset 423504 cbc9ee71c55fe67197ffc3b51765fc085c075ae9
parent 423499 967cfe6d43521d4c930884eaf3d6e9d8d8ec45f9
child 533467 320b1130c7ff4c7ebe32af1c10a11c05771eba16
push id31922
push usermdeboer@mozilla.com
push dateTue, 11 Oct 2016 09:19:30 +0000
reviewersjaws
bugs1279652
milestone52.0a1
Bug 1279652 - reddit.com invalidates all references in the TextLayer, resulting in zero ClientRects for ranges that we have in the FinderHighlighter cache. This fix makes sure to re-fetch all the ranges when this is detected. r?jaws MozReview-Commit-ID: 8OEKmKn9Nwc
toolkit/modules/FinderHighlighter.jsm
--- a/toolkit/modules/FinderHighlighter.jsm
+++ b/toolkit/modules/FinderHighlighter.jsm
@@ -148,29 +148,32 @@ FinderHighlighter.prototype = {
     this._iterator = Cu.import("resource://gre/modules/FinderIterator.jsm", null).FinderIterator;
     return this._iterator;
   },
 
   /**
    * Each window is unique, globally, and the relation between an active
    * highlighting session and a window is 1:1.
    * For each window we track a number of properties which _at least_ consist of
+   *  - {Boolean} detectedGeometryChange Whether the geometry of the found ranges'
+   *                                     rectangles has changed substantially
    *  - {Set}     dynamicRangesSet       Set of ranges that may move around, depending
    *                                     on page layout changes and user input
    *  - {Map}     frames                 Collection of frames that were encountered
    *                                     when inspecting the found ranges
    *  - {Map}     modalHighlightRectsMap Collection of ranges and their corresponding
    *                                     Rects
    *
    * @param  {nsIDOMWindow} window
    * @return {Object}
    */
   getForWindow(window, propName = null) {
     if (!gWindows.has(window)) {
       gWindows.set(window, {
+        detectedGeometryChange: false,
         dynamicRangesSet: new Set(),
         frames: new Map(),
         modalHighlightRectsMap: new Map(),
         previousRangeRectsCount: 0
       });
     }
     return gWindows.get(window);
   },
@@ -430,18 +433,16 @@ FinderHighlighter.prototype = {
 
       if (data.findAgain)
         dict.updateAllRanges = true;
 
       if (!dict.visible)
         this.show(window);
       else
         this._maybeCreateModalHighlightNodes(window);
-
-      this._updateRangeOutline(dict, textContent);
     }
 
     let outlineNode = dict.modalHighlightOutline;
     if (outlineNode) {
       if (dict.animation)
         dict.animation.finish();
       dict.animation = outlineNode.setAnimationForElement(kModalOutlineId,
         Cu.cloneInto(kModalOutlineAnim.keyframes, window), kModalOutlineAnim.duration);
@@ -858,17 +859,22 @@ FinderHighlighter.prototype = {
    * @return {Set}         Set of rects that were found for the range
    */
   _updateRangeRects(range, checkIfDynamic = true, dict = null) {
     let window = range.startContainer.ownerDocument.defaultView;
     let rects = this._getRangeRects(range, dict);
 
     // Only fetch the rect at this point, if not passed in as argument.
     dict = dict || this.getForWindow(window.top);
+    let oldRects = dict.modalHighlightRectsMap.get(range);
     dict.modalHighlightRectsMap.set(range, rects);
+    // Check here if we suddenly went down to zero rects from more than zero before,
+    // which indicates that we should re-iterate the document.
+    if (oldRects && oldRects.length && !rects.length)
+      dict.detectedGeometryChange = true;
     if (checkIfDynamic && this._isInDynamicContainer(range))
       dict.dynamicRangesSet.add(range);
     return rects;
   },
 
   /**
    * Re-read the rectangles of the ranges that we keep track of separately,
    * because they're enclosed by a position: fixed container DOM node or (i)frame.
@@ -1067,18 +1073,16 @@ FinderHighlighter.prototype = {
       let onVisibilityChange = () => {
         document.removeEventListener("visibilitychange", onVisibilityChange);
         this._maybeCreateModalHighlightNodes(window);
       };
       document.addEventListener("visibilitychange", onVisibilityChange);
       return;
     }
 
-    this._updateRangeOutline(dict);
-
     // Make sure to at least show the dimmed background.
     this._repaintHighlightAllMask(window, false);
   },
 
   /**
    * Build and draw the mask that takes care of the dimmed background that
    * overlays the current page and the mask that cuts out all the rectangles of
    * the ranges that were found.
@@ -1105,28 +1109,33 @@ FinderHighlighter.prototype = {
     if (typeof dict.brightText != "boolean" || dict.updateAllRanges)
       this._detectBrightText(dict);
     let maskStyle = this._getStyleString(kModalStyles.maskNode,
       [ ["width", width + "px"], ["height", height + "px"] ],
       dict.brightText ? kModalStyles.maskNodeBrightText : [],
       paintContent ? kModalStyles.maskNodeTransition : [],
       kDebug ? kModalStyles.maskNodeDebug : []);
     dict.modalHighlightAllMask.setAttributeForElement(kMaskId, "style", maskStyle);
-    if (dict.brightText)
-      dict.modalHighlightAllMask.setAttributeForElement(kMaskId, "brighttext", "true");
+
+    this._updateRangeOutline(dict);
 
     let allRects = [];
     if (paintContent || dict.modalHighlightAllMask) {
-      this._updateRangeOutline(dict);
       this._updateDynamicRangesRects(dict);
 
       let DOMRect = window.DOMRect;
       for (let [range, rects] of dict.modalHighlightRectsMap) {
         if (dict.updateAllRanges)
           rects = this._updateRangeRects(range);
+
+        // If a geometry change was detected, we bail out right away here, because
+        // the current set of ranges has been invalidated.
+        if (dict.detectedGeometryChange)
+          return;
+
         if (this._checkOverlap(dict.currentFoundRange, range))
           continue;
         for (let rect of rects)
           allRects.push(new DOMRect(rect.x, rect.y, rect.width, rect.height));
       }
       dict.updateAllRanges = false;
     }
 
@@ -1196,18 +1205,20 @@ FinderHighlighter.prototype = {
     if (dict.modalRepaintScheduler)
       return;
 
     dict.modalRepaintScheduler = window.setTimeout(() => {
       dict.modalRepaintScheduler = null;
 
       let { width: previousWidth, height: previousHeight } = dict.lastWindowDimensions;
       let { width, height } = dict.lastWindowDimensions = this._getWindowDimensions(window);
-      let pageContentChanged = (Math.abs(previousWidth - width) > kContentChangeThresholdPx ||
+      let pageContentChanged = dict.detectedGeometryChange ||
+                               (Math.abs(previousWidth - width) > kContentChangeThresholdPx ||
                                 Math.abs(previousHeight - height) > kContentChangeThresholdPx);
+      dict.detectedGeometryChange = false;
       // 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;