Bug 1218351 - (e10s) Don't lose initial typed characters when opening the findbar; r=mikedeboer
☠☠ backed out by aa91027325cf ☠ ☠
authorLuis Miguel [:quicksaver] <quicksaver@gmail.com>
Wed, 04 Nov 2015 15:26:40 +0000
changeset 308370 5807dfe3b8736404b267fb6fb883d5cf440a62fe
parent 308369 01dac856fde00a7a8de9af2534fb2d4764196641
child 308371 e23d0660a042e6a00a1de55667cc9b30c63620d7
push id1040
push userraliiev@mozilla.com
push dateMon, 29 Feb 2016 17:11:22 +0000
treeherdermozilla-release@8c3167321162 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer
bugs1218351
milestone45.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 1218351 - (e10s) Don't lose initial typed characters when opening the findbar; r=mikedeboer Moved some of the routine from content to chrome process. The drawback is that now the content process must wait for a response on every keypress.
testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
testing/mochitest/tests/SimpleTest/AsyncUtilsContent.js
toolkit/content/browser-content.js
toolkit/content/tests/browser/browser_findbar.js
toolkit/content/widgets/findbar.xml
--- a/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
+++ b/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm
@@ -27,16 +27,18 @@ Cu.import("resource://testing-common/Tes
 Cc["@mozilla.org/globalmessagemanager;1"]
   .getService(Ci.nsIMessageListenerManager)
   .loadFrameScript(
     "chrome://mochikit/content/tests/BrowserTestUtils/content-utils.js", true);
 
 XPCOMUtils.defineLazyModuleGetter(this, "E10SUtils",
   "resource:///modules/E10SUtils.jsm");
 
+var gSendCharCount = 0;
+
 this.BrowserTestUtils = {
   /**
    * Loads a page in a new tab, executes a Task and closes the tab.
    *
    * @param options
    *        An object with the following properties:
    *        {
    *          gBrowser:
@@ -658,17 +660,26 @@ this.BrowserTestUtils = {
    * @param {Browser} browser
    *        Browser element, must not be null.
    *
    * @returns {Promise}
    * @resolves True if the keypress event was synthesized.
    */
   sendChar(char, browser) {
     return new Promise(resolve => {
+      let seq = ++gSendCharCount;
       let mm = browser.messageManager;
+
       mm.addMessageListener("Test:SendCharDone", function charMsg(message) {
+        if (message.data.seq != seq)
+          return;
+
         mm.removeMessageListener("Test:SendCharDone", charMsg);
         resolve(message.data.sendCharResult);
       });
-      mm.sendAsyncMessage("Test:SendChar", { char: char });
+
+      mm.sendAsyncMessage("Test:SendChar", {
+        char: char,
+        seq: seq
+      });
     });
   }
 };
--- a/testing/mochitest/tests/SimpleTest/AsyncUtilsContent.js
+++ b/testing/mochitest/tests/SimpleTest/AsyncUtilsContent.js
@@ -48,10 +48,13 @@ addMessageListener("Test:SynthesizeMouse
   }
 
   let result = EventUtils.synthesizeMouseAtPoint(left, top, data.event, content);
   sendAsyncMessage("Test:SynthesizeMouseDone", { defaultPrevented: result });
 });
 
 addMessageListener("Test:SendChar", message => {
   let result = EventUtils.sendChar(message.data.char, content);
-  sendAsyncMessage("Test:SendCharDone", { sendCharResult: result });
+  sendAsyncMessage("Test:SendCharDone", {
+    sendCharResult: result,
+    seq: message.data.seq
+  });
 });
--- a/toolkit/content/browser-content.js
+++ b/toolkit/content/browser-content.js
@@ -575,35 +575,29 @@ addMessageListener("SwitchDocumentDirect
   SwitchDocumentDirection(content.window);
 });
 
 var FindBar = {
   /* Please keep in sync with toolkit/content/widgets/findbar.xml */
   FIND_NORMAL: 0,
   FIND_TYPEAHEAD: 1,
   FIND_LINKS: 2,
-  FAYT_LINKS_KEY: "'".charCodeAt(0),
-  FAYT_TEXT_KEY: "/".charCodeAt(0),
 
   _findMode: 0,
-  _findAsYouType: false,
 
   init() {
-    this._findAsYouType =
-      Services.prefs.getBoolPref("accessibility.typeaheadfind");
     addMessageListener("Findbar:UpdateState", this);
     Services.els.addSystemEventListener(global, "keypress", this, false);
     Services.els.addSystemEventListener(global, "mouseup", this, false);
   },
 
   receiveMessage(msg) {
     switch (msg.name) {
       case "Findbar:UpdateState":
         this._findMode = msg.data.findMode;
-        this._findAsYouType = msg.data.findAsYouType;
         break;
     }
   },
 
   handleEvent(event) {
     switch (event.type) {
       case "keypress":
         this._onKeypress(event);
@@ -627,35 +621,31 @@ var FindBar = {
     return BrowserUtils.shouldFastFind(elt, win);
   },
 
   _onKeypress(event) {
     // Useless keys:
     if (event.ctrlKey || event.altKey || event.metaKey || event.defaultPrevented) {
       return;
     }
-    // Not interested in random keypresses most of the time:
-    if (this._findMode == this.FIND_NORMAL && !this._findAsYouType &&
-        event.charCode != this.FAYT_LINKS_KEY && event.charCode != this.FAYT_TEXT_KEY) {
-      return;
-    }
 
     // Check the focused element etc.
-    if (!this._shouldFastFind()) {
-      return;
-    }
+    let shouldFastFind = this._shouldFastFind();
 
     let fakeEvent = {};
     for (let k in event) {
       if (typeof event[k] != "object" && typeof event[k] != "function") {
         fakeEvent[k] = event[k];
       }
     }
     // sendSyncMessage returns an array of the responses from all listeners
-    let rv = sendSyncMessage("Findbar:Keypress", fakeEvent);
+    let rv = sendSyncMessage("Findbar:Keypress", {
+      fakeEvent: fakeEvent,
+      shouldFastFind: shouldFastFind
+    });
     if (rv.indexOf(false) !== -1) {
       event.preventDefault();
       return false;
     }
   },
 
   _onMouseup(event) {
     if (this._findMode != this.FIND_NORMAL)
--- a/toolkit/content/tests/browser/browser_findbar.js
+++ b/toolkit/content/tests/browser/browser_findbar.js
@@ -153,16 +153,55 @@ add_task(function * test_reinitializatio
      "Findbar status text should be 'Phrase not found'");
 
   yield promiseFindFinished("s", false);
   ok(!findbar._findStatusDesc.textContent, "Findbar status should be empty");
 
   yield BrowserTestUtils.removeTab(tab);
 });
 
+/**
+ * Ensure that the initial typed characters aren't lost immediately after
+ * opening the find bar.
+ */
+add_task(function* () {
+  // This test only makes sence in e10s evironment.
+  if (!gMultiProcessBrowser) {
+    info("Skipping this test because of non-e10s environment.");
+    return true;
+  }
+
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_PAGE_URI);
+  let browser = tab.linkedBrowser;
+
+  ok(!gFindBarInitialized, "findbar isn't initialized yet");
+
+  let findBar = gFindBar;
+  let initialValue = findBar._findField.value;
+
+  EventUtils.synthesizeKey("f", { accelKey: true }, window);
+
+  let promises = [
+    BrowserTestUtils.sendChar("a", browser),
+    BrowserTestUtils.sendChar("b", browser),
+    BrowserTestUtils.sendChar("c", browser)
+  ];
+
+  isnot(document.activeElement, findBar._findField.inputField,
+    "findbar is not yet focused");
+  is(findBar._findField.value, initialValue, "still has initial find query");
+
+  yield Promise.all(promises);
+  is(document.activeElement, findBar._findField.inputField,
+    "findbar is now focused");
+  is(findBar._findField.value, "abc", "abc fully entered as find query");
+
+  yield BrowserTestUtils.removeTab(tab);
+});
+
 function promiseFindFinished(searchText, highlightOn) {
   let deferred = Promise.defer();
 
   let findbar = gBrowser.getFindBar();
   findbar.startFind(findbar.FIND_NORMAL);
   let highlightElement = findbar.getElement("highlight");
   if (highlightElement.checked != highlightOn)
     highlightElement.click();
--- a/toolkit/content/widgets/findbar.xml
+++ b/toolkit/content/widgets/findbar.xml
@@ -280,17 +280,17 @@
             }
             let finder = this._browser.finder;
             if (finder)
               finder.removeResultListener(this);
           }
 
           this._browser = val;
           if (this._browser) {
-            // Need to do this in case FAYT
+            // Need to do this to ensure the correct initial state.
             this._updateBrowserWithState();
             this._browser.messageManager.addMessageListener("Findbar:Keypress", this);
             this._browser.messageManager.addMessageListener("Findbar:Mouseup", this);
             this._browser.finder.addResultListener(this);
 
             this._findField.value = this._browser._lastSearchString;
             this.toggleHighlight(this.browser._lastSearchHighlight);
           }
@@ -315,17 +315,16 @@
             return;
 
           let prefsvc =
             aSubject.QueryInterface(Components.interfaces.nsIPrefBranch);
 
           switch (aPrefName) {
             case "accessibility.typeaheadfind":
               this._self._findAsYouType = prefsvc.getBoolPref(aPrefName);
-              this._self._updateBrowserWithState();
               break;
             case "accessibility.typeaheadfind.linksonly":
               this._self._typeAheadLinksOnly = prefsvc.getBoolPref(aPrefName);
               break;
             case "accessibility.typeaheadfind.casesensitive":
               this._self._setCaseSensitivity(prefsvc.getIntPref(aPrefName));
               break;
           }
@@ -736,30 +735,44 @@
 
       <!-- We get a fake event object through an IPC message which contains the
            data we need to make a decision. We then return |true| if and only if
            the page gets to deal with the event itself. Everywhere we return
            false, the message sender will take care of calling event.preventDefault
            on the real event. -->
       <method name="_onBrowserKeypress">
         <parameter name="aFakeEvent"/>
+        <parameter name="aShouldFastFind"/>
         <body><![CDATA[
           const FAYT_LINKS_KEY = "'";
           const FAYT_TEXT_KEY = "/";
 
+          // Fast keypresses can stack up when the content process is slow or
+          // hangs when in e10s mode. We make sure the findbar isn't 'opened'
+          // several times in a row, because then the find query is selected
+          // each time, losing characters typed initially.
+          let inputField = this._findField.inputField;
+          if (!this.hidden && document.activeElement == inputField) {
+            this._dispatchKeypressEvent(inputField, aFakeEvent);
+            return false;
+          }
+
           if (this._findMode != this.FIND_NORMAL && this._quickFindTimeout) {
             if (!aFakeEvent.charCode)
               return true;
 
             this._findField.select();
             this._findField.focus();
             this._dispatchKeypressEvent(this._findField.inputField, aFakeEvent);
             return false;
           }
 
+          if (!aShouldFastFind)
+            return true;
+
           let key = aFakeEvent.charCode ? String.fromCharCode(aFakeEvent.charCode) : null;
           let manualstartFAYT = (key == FAYT_LINKS_KEY || key == FAYT_TEXT_KEY);
           let autostartFAYT = !manualstartFAYT && this._findAsYouType &&
                               key && key != " ";
           if (manualstartFAYT || autostartFAYT) {
             let mode = (key == FAYT_LINKS_KEY ||
                         (autostartFAYT && this._typeAheadLinksOnly)) ?
               this.FIND_LINKS : this.FIND_TYPEAHEAD;
@@ -792,27 +805,27 @@
           }
           switch (aMessage.name) {
             case "Findbar:Mouseup":
               if (!this.hidden && this._findMode != this.FIND_NORMAL)
                 this.close();
               break;
 
             case "Findbar:Keypress":
-              return this._onBrowserKeypress(aMessage.data);
+              return this._onBrowserKeypress(aMessage.data.fakeEvent,
+                                             aMessage.data.shouldFastFind);
           }
         ]]></body>
       </method>
 
       <method name="_updateBrowserWithState">
         <body><![CDATA[
           if (this._browser && this._browser.messageManager) {
             this._browser.messageManager.sendAsyncMessage("Findbar:UpdateState", {
-              findMode: this._findMode,
-              findAsYouType: this._findAsYouType,
+              findMode: this._findMode
             });
           }
         ]]></body>
       </method>
 
       <method name="_enableFindButtons">
         <parameter name="aEnable"/>
         <body><![CDATA[