Bug 921308: don't scroll when an element is focused on findbar close. r=evilpie
authorMike de Boer <mdeboer@mozilla.com>
Wed, 16 Oct 2013 17:04:49 +0200
changeset 164796 3e103997950b87db99481a54b3bbe942ca6b0b9f
parent 164795 ae104231ede7d3c54b867b2a39238c7370ce1455
child 164797 aa160d21eb2582af3699a218e3f67451df71a921
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs921308
milestone27.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 921308: don't scroll when an element is focused on findbar close. r=evilpie
toolkit/content/widgets/findbar.xml
toolkit/modules/Finder.jsm
toolkit/modules/RemoteFinder.jsm
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -1133,15 +1133,38 @@
           else
             this._findFailedString = null;
 
           if (this._findMode != this.FIND_NORMAL && !this.hidden)
             this._setFindCloseTimeout();
         ]]></body>
       </method>
 
+      <!--
+        - This handler may cancel a request to focus content by returning |false|
+        - explicitly.
+        -->
+      <method name="shouldFocusContent">
+        <body><![CDATA[
+          const fm = Components.classes["@mozilla.org/focus-manager;1"]
+                               .getService(Components.interfaces.nsIFocusManager);
+          if (fm.focusedWindow != window)
+            return false;
+
+          let focusedElement = fm.focusedElement;
+          if (!focusedElement)
+            return false;
+
+          let bindingParent = document.getBindingParent(focusedElement);
+          if (bindingParent != this && bindingParent != this._findField)
+            return false;
+
+          return true;
+        ]]></body>
+      </method>
+
     </implementation>
 
     <handlers>
       <handler event="keypress" keycode="VK_ESCAPE" phase="capturing" action="this.close();" preventdefault="true"/>
     </handlers>
   </binding>
 </bindings>
--- a/toolkit/modules/Finder.jsm
+++ b/toolkit/modules/Finder.jsm
@@ -111,24 +111,32 @@ Finder.prototype = {
 
     fastFind.collapseSelection();
     this.enableSelection();
 
     this._restoreOriginalOutline();
   },
 
   focusContent: function() {
+    // Allow Finder listeners to cancel focusing the content.
+    for (let l of this._listeners) {
+      if (!l.shouldFocusContent())
+        return;
+    }
+
     let fastFind = this._fastFind;
-
+    const fm = Cc["@mozilla.org/focus-manager;1"].getService(Ci.nsIFocusManager);
     try {
-      // Try to find the best possible match that should receive focus.
+      // Try to find the best possible match that should receive focus and
+      // block scrolling on focus since find already scrolls. Further
+      // scrolling is due to user action, so don't override this.
       if (fastFind.foundLink) {
-        fastFind.foundLink.focus();
+        fm.setFocus(fastFind.foundLink, fm.FLAG_NOSCROLL);
       } else if (fastFind.foundEditable) {
-        fastFind.foundEditable.focus();
+        fm.setFocus(fastFind.foundEditable, fm.FLAG_NOSCROLL);
         fastFind.collapseSelection();
       } else {
         this._getWindow().focus()
       }
     } catch (e) {}
   },
 
   keyPress: function (aEvent) {
--- a/toolkit/modules/RemoteFinder.jsm
+++ b/toolkit/modules/RemoteFinder.jsm
@@ -109,16 +109,22 @@ RemoteFinderListener.prototype = {
   ],
 
   onFindResult: function (aResult, aFindBackwards, aLinkURL) {
     let data = { result: aResult, findBackwards: aFindBackwards,
                  linkURL: aLinkURL, searchString: this._finder.searchString };
     this._global.sendAsyncMessage("Finder:Result", data);
   },
 
+  //XXXmikedeboer-20131016: implement |shouldFocusContent| here to mitigate
+  //                        issues like bug 921338 and bug 921308.
+  shouldFocusContent: function () {
+    return true;
+  },
+
   receiveMessage: function (aMessage) {
     let data = aMessage.data;
 
     switch (aMessage.name) {
       case "Finder:CaseSensitive":
         this._finder.caseSensitive = data.caseSensitive;
         break;