Bug 279022 - "Highlight all" cannot be undone after switching tabs. r=mano
authorGraeme McCutcheon <graememcc_firefox@graeme-online.co.uk>
Tue, 19 May 2009 16:20:19 +0100
changeset 28586 e4cef2105e3e742e292d7c0e160dd67e0d83745f
parent 28585 eeb44ecd8038443d76aaf6a43ee8b5d5268a49f4
child 28587 8d3c4c7122eb7df18967fc7a1dfa13b1b5262ece
push idunknown
push userunknown
push dateunknown
reviewersmano
bugs279022
milestone1.9.2a1pre
Bug 279022 - "Highlight all" cannot be undone after switching tabs. r=mano
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/Makefile.in
browser/base/content/test/browser_bug279022.js
toolkit/content/widgets/findbar.xml
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -4094,22 +4094,16 @@ var XULBrowserWindow = {
     }
     UpdateBackForwardCommands(gBrowser.webNavigation);
 
     if (gFindBar.findMode != gFindBar.FIND_NORMAL) {
       // Close the Find toolbar if we're in old-style TAF mode
       gFindBar.close();
     }
 
-    // XXXmano new-findbar, do something useful once it lands.
-    // Of course, this is especially wrong with bfcache on...
-
-    // fix bug 253793 - turn off highlight when page changes
-    gFindBar.getElement("highlight").checked = false;
-
     // See bug 358202, when tabs are switched during a drag operation,
     // timers don't fire on windows (bug 203573)
     if (aRequest)
       setTimeout(function () { XULBrowserWindow.asyncUpdateUI(); }, 0);
     else
       this.asyncUpdateUI();
   },
   
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1856,18 +1856,19 @@
           <![CDATA[
             for (var i = 0; i < this.mProgressListeners.length; i++) {
               if (this.mProgressListeners[i] == aListener) {
                 this.mProgressListeners.splice(i, 1);
                 break;
               }
             }
 
-            if (!this.mTabbedMode)
-              // Don't forget to remove it from the filter we hooked it up to
+            // If we have hooked it up to a filter, don't forget to remove it
+            // (but don't fail if it wasn't)
+            if (!this.mTabbedMode && this.mTabFilters[0])
               this.mTabFilters[0].removeProgressListener(aListener);
          ]]>
         </body>
       </method>
 
       <method name="addTabsProgressListener">
         <parameter name="aListener"/>
         <body>
--- a/browser/base/content/test/Makefile.in
+++ b/browser/base/content/test/Makefile.in
@@ -64,16 +64,17 @@ include $(topsrcdir)/config/rules.mk
 #   browser_bug423833.js is bug 428712
 #   browser_sanitize-download-history.js is bug 432425
 #
 # browser_sanitizeDialog_treeView.js is disabled until the tree view is added
 # back to the clear recent history dialog (santize.xul), if it ever is (bug
 # 480169)
 
 _BROWSER_FILES = browser_sanitize-timespans.js \
+		 browser_bug279022.js \
                  browser_bug405137.js \
                  browser_bug409481.js \
                  browser_bug413915.js \
                  browser_bug419612.js \
                  browser_bug420160.js \
                  browser_bug424101.js \
                  browser_bug427559.js \
                  browser_bug432599.js \
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/browser_bug279022.js
@@ -0,0 +1,107 @@
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Test Code for bug 279022.
+ *
+ * The Initial Developer of the Original Code is
+ * Graeme McCutcheon <graememcc_firefox@graeme-online.co.uk>.
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either of the GNU General Public License Version 2 or later (the "GPL"),
+ * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+let testPage1 = 'data:text/html,<p>Mozilla</p>';
+let testPage2 = 'data:text/html,<p>Firefox</p>';
+let testPage3 = 'data:text/html,<p>Seamonkey</p><input type="text" value="Gecko"></input>';
+let hButton = gFindBar.getElement("highlight");
+
+var testTab1, testTab2, testTab3;
+var testBrowser1, testBrowser2, testBrowser3;
+
+function test() {
+  waitForExplicitFinish();
+
+  testTab1 = gBrowser.addTab();
+  testTab2 = gBrowser.addTab();
+  testTab3 = gBrowser.addTab();
+  testBrowser1 = gBrowser.getBrowserForTab(testTab1);
+  testBrowser2 = gBrowser.getBrowserForTab(testTab2);
+  testBrowser3 = gBrowser.getBrowserForTab(testTab3);
+
+  // Load the test documents into the relevant browsers
+  testBrowser3.loadURI(testPage3);
+  testBrowser2.loadURI(testPage2);
+  testBrowser1.addEventListener("load", testTabs, true);
+  testBrowser1.loadURI(testPage1);
+}
+
+function testTabs() {
+  testBrowser1.removeEventListener("load", testTabs, true);
+
+  // Select the tab and create some highlighting
+  gBrowser.selectedTab = testTab1;
+  gFindBar.getElement("find-case-sensitive").checked = false;
+  gFindBar._highlightDoc(true, "Mozilla");
+
+  // Test 1: switch tab and check highlight button deselected
+  gBrowser.selectedTab = testTab2;
+  ok(!hButton.checked, "highlight button deselected after changing tab");
+  gFindBar._findField.value = "";
+
+  // Test 2: switch back to tab with highlighting
+  gBrowser.selectedTab = testTab1;
+  ok(hButton.checked, "highlight button re-enabled on tab with highlighting");
+  var searchTermOK = gFindBar._findField.value == "Mozilla";
+  ok(searchTermOK, "detected correct search term");
+
+  // Create highlighting where match is in an editable element
+  gBrowser.selectedTab = testTab3;
+  gFindBar._highlightDoc(true, "Gecko");
+
+  // Test 4: Switch to tab without highlighting again
+  gBrowser.selectedTab = testTab2;
+  ok(!hButton.checked, "highlight button deselected again");
+  gFindBar._findField.value = "";
+
+  // Test 5: Switch to tab with highlighting in editable element
+  gBrowser.selectedTab = testTab3;
+  ok(hButton.checked, "highlighting detected in editable element");
+  searchTermOK = gFindBar._findField.value == "Gecko";
+  ok(searchTermOK, "detected correct search term");
+
+  // Test 6: Switch between two tabs with highlighting - search term changed?
+  gBrowser.selectedTab = testTab1;
+  searchTermOK = gFindBar._findField.value == "Mozilla";
+  ok(searchTermOK, "correctly changed search term");
+
+  gBrowser.removeTab(testTab3);
+  gBrowser.removeTab(testTab2);
+  gBrowser.removeTab(testTab1);
+  finish();
+}
+
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -275,17 +275,17 @@
       <xul:image anonid="find-status-icon" class="findbar-find-fast find-status-icon"/>
       <xul:description anonid="find-status" class="findbar-find-fast findbar-find-status"
                        control="findbar-textbox">
       <!-- Do not use value, first child is used because it provides a11y with text change events -->
       </xul:description>
     </xul:hbox>
     </content>
 
-    <implementation implements="nsIDOMEventListener, nsIEditActionListener">
+    <implementation implements="nsIDOMEventListener, nsIEditActionListener, nsIWebProgressListener">
       <field name="FIND_NORMAL">0</field>
       <field name="FIND_TYPEAHEAD">1</field>
       <field name="FIND_LINKS">2</field>
 
       <field name="_findMode">0</field>
       <field name="_tmpOutline">null</field>
       <field name="_tmpOutlineOffset">"0"</field>
       <field name="_drawOutline">false</field>
@@ -322,25 +322,32 @@
               document.getElementById(this.getAttribute("browserid"));
           }
           return this._browser;
         ]]></getter>
         <setter><![CDATA[
           if (this._browser) {
             this._browser.removeEventListener("keypress", this, false);
             this._browser.removeEventListener("mouseup", this, false);
+            try {
+              // don't fail when the browser has been supplied through the
+              // browserid attribute and we've not yet hooked up the listener
+              this._browser.removeProgressListener(this);
+            } catch (e) {}
             this._foundLink = null;
             this._foundEditable = null;
             this._currentWindow = null;
           }
 
           this._browser = val;
           if (this._browser) {
             this._browser.addEventListener("keypress", this, false);
             this._browser.addEventListener("mouseup", this, false);
+            this._browser.addProgressListener(this,
+              Components.interfaces.nsIWebProgress.NOTIFY_LOCATION);
             this._findField.value = this._browser.fastFind.searchString;
           }
           return val;
         ]]></setter>
       </property>
 
       <field name="_observer"><![CDATA[({
         _self: this,
@@ -886,16 +893,134 @@
             // Unimplemented
             notifyDocumentCreated: function() {},
             notifyDocumentStateChanged: function(aDirty) {}
           });
         ]]></body>
       </method>
 
       <!--
+        - nsIWebProgressListener logic follows
+        - 
+        - We implement this interface to allow us to update our status UI 
+        - when navigating to a new tab, or loading a page from the cache.
+        - This then allows the user to adjust the highlighting on the newly
+        - displayed page.
+        -->
+
+      <!--
+        - Detects whether highlighting is present in a given window,
+        - by checking the window's content document, and recursively checking
+        - any frames contained within this window.
+        - @param aWindow
+        -        the window to check
+        - @returns the highlighted string, if there is highlighting present,
+        -          null otherwise
+        -->
+      <method name="_getHighlightedString">
+        <parameter name="aWindow"/>
+        <body><![CDATA[
+          var win = aWindow || this.browser.contentWindow;
+
+          // Obtain the selection controller for this window
+          var controller = this._getSelectionController(win);
+
+          if (!controller)
+            return null;
+
+          var selection = controller.getSelection(controller.SELECTION_FIND);
+          if (selection && selection.rangeCount > 0)
+            return selection.getRangeAt(0).toString();
+
+          var doc = win.document;
+          // Bail now if document cannot contain highlighting
+          if (!doc || !(doc instanceof HTMLDocument))
+            return null;
+
+          // Check for highlighting in elements with independent
+          // selection controllers
+          if (this._editors) {
+            for (var x = 0; x < this._editors.length; x++) {
+              if (this._editors[x].document == doc) {
+                var sel = this._editors[x].selectionController
+                                          .getSelection(this._findSelection);
+                return sel.getRangeAt(0).toString();
+              }
+            }
+          }
+
+          // If we still haven't found highlighting, recursively check any
+          // frames in the current window
+          var hasHighlight = null;
+          var i = 0;
+          while (!hasHighlight && win.frames && i < win.frames.length) {
+            hasHighlight = this._getHighlightedString(win.frames[i]);
+            i++;
+          }
+          return hasHighlight;
+        ]]></body>
+      </method>
+
+      <!-- Start of nsIWebProgressListener implementations -->
+
+      <!--
+        - Updates the findbar UI on a page change, to allow removal of highlighting
+        - where appropriate
+        -->
+      <method name="onLocationChange">
+        <parameter name="aWebProgress"/>
+        <parameter name="aRequest"/>
+        <parameter name="aURI"/>
+        <body><![CDATA[
+          var highlightButton = this.getElement("highlight");
+
+          // Establish whether any highlighting exists on the new page
+          var lastHighlightString = this._getHighlightedString();
+          if (!lastHighlightString) {
+            highlightButton.checked = false;
+            if (!this._findField.value)
+              highlightButton.disabled = true;
+            return;
+          }
+
+          // Update the highlight button and display the
+          // currently highlighted string
+          highlightButton.checked = true;
+          highlightButton.disabled = false;
+          this._findField.value = lastHighlightString;
+        ]]></body>
+      </method>
+
+      <method name="onProgressChange">
+        <body><![CDATA[
+          // Unimplemented
+        ]]></body>
+      </method>
+
+      <method name="onSecurityChange">
+        <body><![CDATA[
+          // Unimplemented
+        ]]></body>
+      </method>
+
+      <method name="onStateChange">
+        <body><![CDATA[
+          // Unimplemented
+        ]]></body>
+      </method>
+
+      <method name="onStatusChange">
+        <body><![CDATA[
+          // Unimplemented
+        ]]></body>
+      </method>
+
+      <!-- End of nsIWebProgressListener implementations -->
+
+      <!--
         - Turns highlight on or off.
         - @param aHighlight (boolean)
         -        Whether to turn the highlight on or off
         -->
       <method name="toggleHighlight">
         <parameter name="aHighlight"/>
         <body><![CDATA[
           var word = this._findField.value;