Bug 1304497 - always draw all rectangles, because the showing and hiding of overlapping rectangles during find occurrence navigation is jarring. This essentially backs out bug 1300824. r=jaws
authorMike de Boer <mdeboer@mozilla.com>
Wed, 02 Nov 2016 22:18:15 +0100
changeset 363654 7e98ff6fdd6405c8802b4aa3ee754d90e9a9b1b3
parent 363653 fd1f1513e91e805ff87c266242a80c2962883e9b
child 363655 47cf46bd3e3ebd91921def20038d2989d88d4a43
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-beta@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws
bugs1304497, 1300824
milestone52.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 1304497 - always draw all rectangles, because the showing and hiding of overlapping rectangles during find occurrence navigation is jarring. This essentially backs out bug 1300824. r=jaws MozReview-Commit-ID: 32HJaR0czyh
toolkit/modules/FinderHighlighter.jsm
toolkit/modules/tests/browser/browser_FinderHighlighter.js
--- a/toolkit/modules/FinderHighlighter.jsm
+++ b/toolkit/modules/FinderHighlighter.jsm
@@ -655,21 +655,19 @@ FinderHighlighter.prototype = {
   /**
    * Utility; fetch the current text contents of a given range.
    *
    * @param  {nsIDOMRange} range Range object to extract the contents from.
    * @return {Array} Snippets of text.
    */
   _getRangeContentArray(range) {
     let content = range.cloneContents();
-    let t, textContent = [];
+    let textContent = [];
     for (let node of content.childNodes) {
-      t = node.textContent || node.nodeValue;
-      // if (t && t.trim())
-        textContent.push(t);
+      textContent.push(node.textContent || node.nodeValue);
     }
     return textContent;
   },
 
   /**
    * Utility; get all available font styles as applied to the content of a given
    * range. The CSS properties we look for can be found in `kFontPropsCSS`.
    *
@@ -1140,18 +1138,16 @@ FinderHighlighter.prototype = {
         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;
     }
 
     dict.modalHighlightAllMask.setCutoutRectsForElement(kMaskId, allRects);
   },
--- a/toolkit/modules/tests/browser/browser_FinderHighlighter.js
+++ b/toolkit/modules/tests/browser/browser_FinderHighlighter.js
@@ -179,41 +179,41 @@ add_task(function* setup() {
     [kPrefModalHighlight, true]
   ]});
 });
 
 // Test the results of modal highlighting, which is on by default.
 add_task(function* testModalResults() {
   let tests = new Map([
     ["Roland", {
-      rectCount: 1,
+      rectCount: 2,
       insertCalls: [2, 4],
       removeCalls: [0, 1]
     }],
     ["their law might propagate their kind", {
-      rectCount: 0,
-      insertCalls: [28, 31],
-      removeCalls: [28, 30],
+      rectCount: 2,
+      insertCalls: [5, 6],
+      removeCalls: [4, 5],
       extraTest: function(maskNode, outlineNode, rects) {
         Assert.equal(outlineNode.getElementsByTagName("div").length, 2,
           "There should be multiple rects drawn");
       }
     }],
     ["ro", {
-      rectCount: 40,
+      rectCount: 41,
       insertCalls: [1, 4],
       removeCalls: [1, 3]
     }],
     ["new", {
-      rectCount: 1,
+      rectCount: 2,
       insertCalls: [1, 4],
       removeCalls: [0, 2]
     }],
     ["o", {
-      rectCount: 491,
+      rectCount: 492,
       insertCalls: [1, 4],
       removeCalls: [0, 2]
     }]
   ]);
   let url = kFixtureBaseURL + "file_FinderSample.html";
   yield BrowserTestUtils.withNewTab(url, function* (browser) {
     let findbar = gBrowser.getFindBar();
 
@@ -244,17 +244,17 @@ add_task(function* testModalSwitching() 
   yield BrowserTestUtils.withNewTab(url, function* (browser) {
     let findbar = gBrowser.getFindBar();
 
     yield promiseOpenFindbar(findbar);
     Assert.ok(!findbar.hidden, "Findbar should be open now.");
 
     let word = "Roland";
     let expectedResult = {
-      rectCount: 1,
+      rectCount: 2,
       insertCalls: [2, 4],
       removeCalls: [0, 1]
     };
     let promise = promiseTestHighlighterOutput(browser, word, expectedResult);
     yield promiseEnterStringIntoFindField(findbar, word);
     yield promise;
 
     yield SpecialPowers.pushPrefEnv({ "set": [[ kPrefModalHighlight, false ]] });
@@ -280,37 +280,38 @@ add_task(function* testDarkPageDetection
   let url = kFixtureBaseURL + "file_FinderSample.html";
   yield BrowserTestUtils.withNewTab(url, function* (browser) {
     let findbar = gBrowser.getFindBar();
 
     yield promiseOpenFindbar(findbar);
 
     let word = "Roland";
     let expectedResult = {
-      rectCount: 1,
+      rectCount: 2,
       insertCalls: [1, 3],
       removeCalls: [0, 1]
     };
     let promise = promiseTestHighlighterOutput(browser, word, expectedResult, function(node) {
-      Assert.ok(!node.hasAttribute("brighttext"), "White HTML page shouldn't have 'brighttext' set");
+      Assert.ok(node.style.background.startsWith("rgba(0, 0, 0"),
+        "White HTML page should have a black background color set for the mask");
     });
     yield promiseEnterStringIntoFindField(findbar, word);
     yield promise;
 
     findbar.close(true);
   });
 
   yield BrowserTestUtils.withNewTab(url, function* (browser) {
     let findbar = gBrowser.getFindBar();
 
     yield promiseOpenFindbar(findbar);
 
     let word = "Roland";
     let expectedResult = {
-      rectCount: 1,
+      rectCount: 2,
       insertCalls: [2, 4],
       removeCalls: [0, 1]
     };
 
     yield ContentTask.spawn(browser, null, function* () {
       let dwu = content.QueryInterface(Ci.nsIInterfaceRequestor)
         .getInterface(Ci.nsIDOMWindowUtils);
       let uri = "data:text/css;charset=utf-8," + encodeURIComponent(`
@@ -319,17 +320,18 @@ add_task(function* testDarkPageDetection
           color: white;
         }`);
       try {
         dwu.loadSheetUsingURIString(uri, dwu.USER_SHEET);
       } catch (e) {}
     });
 
     let promise = promiseTestHighlighterOutput(browser, word, expectedResult, node => {
-      Assert.ok(node.hasAttribute("brighttext"), "Dark HTML page should have 'brighttext' set");
+      Assert.ok(node.style.background.startsWith("rgba(255, 255, 255"),
+        "Dark HTML page should have a white background color set for the mask");
     });
     yield promiseEnterStringIntoFindField(findbar, word);
     yield promise;
 
     findbar.close(true);
   });
 });
 
@@ -337,17 +339,17 @@ add_task(function* testHighlightAllToggl
   let url = kFixtureBaseURL + "file_FinderSample.html";
   yield BrowserTestUtils.withNewTab(url, function* (browser) {
     let findbar = gBrowser.getFindBar();
 
     yield promiseOpenFindbar(findbar);
 
     let word = "Roland";
     let expectedResult = {
-      rectCount: 1,
+      rectCount: 2,
       insertCalls: [2, 4],
       removeCalls: [0, 1]
     };
     let promise = promiseTestHighlighterOutput(browser, word, expectedResult);
     yield promiseEnterStringIntoFindField(findbar, word);
     yield promise;
 
     // We now know we have multiple rectangles highlighted, so it's a good time
@@ -358,17 +360,17 @@ add_task(function* testHighlightAllToggl
       removeCalls: [0, 1]
     };
     promise = promiseTestHighlighterOutput(browser, word, expectedResult);
     yield SpecialPowers.pushPrefEnv({ "set": [[ kHighlightAllPref, false ]] });
     yield promise;
 
     // For posterity, let's switch back.
     expectedResult = {
-      rectCount: 1,
+      rectCount: 2,
       insertCalls: [1, 3],
       removeCalls: [0, 1]
     };
     promise = promiseTestHighlighterOutput(browser, word, expectedResult);
     yield SpecialPowers.pushPrefEnv({ "set": [[ kHighlightAllPref, true ]] });
     yield promise;
   });
 });
@@ -403,17 +405,17 @@ add_task(function* testHideOnLocationCha
   let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, url);
   let browser = tab.linkedBrowser;
   let findbar = gBrowser.getFindBar();
 
   yield promiseOpenFindbar(findbar);
 
   let word = "Roland";
   let expectedResult = {
-    rectCount: 1,
+    rectCount: 2,
     insertCalls: [2, 4],
     removeCalls: [0, 1]
   };
   let promise = promiseTestHighlighterOutput(browser, word, expectedResult);
   yield promiseEnterStringIntoFindField(findbar, word);
   yield promise;
 
   // Now we try to navigate away! (Using the same page)
@@ -431,19 +433,19 @@ add_task(function* testHideOnLocationCha
 add_task(function* testHideOnClear() {
   let url = kFixtureBaseURL + "file_FinderSample.html";
   yield BrowserTestUtils.withNewTab(url, function* (browser) {
     let findbar = gBrowser.getFindBar();
     yield promiseOpenFindbar(findbar);
 
     let word = "Roland";
     let expectedResult = {
-      rectCount: 1,
+      rectCount: 2,
       insertCalls: [2, 4],
-      removeCalls: [1, 2]
+      removeCalls: [0, 2]
     };
     let promise = promiseTestHighlighterOutput(browser, word, expectedResult);
     yield promiseEnterStringIntoFindField(findbar, word);
     yield promise;
 
     yield new Promise(resolve => setTimeout(resolve, kIteratorTimeout));
     promise = promiseTestHighlighterOutput(browser, "", {
       rectCount: 0,