Bug 1105967 - Typing fast in address bar and pressing enter leads to missing end characters. a=paolo
authorBlair McBride <bmcbride@mozilla.com>
Tue, 24 Feb 2015 16:38:25 +1300
changeset 230448 6c9f72338190bd156278ef11c01e87a134f91732
parent 230447 0f792fdfedd65d758c43eb11d9f7aeb080efa9d4
child 230449 4c55a705e62943197fa1e5fc7db20c6af1548d6b
push id28325
push usercbook@mozilla.com
push dateTue, 24 Feb 2015 12:19:29 +0000
treeherdermozilla-central@b74938ef94bc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaolo
bugs1105967
milestone39.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 1105967 - Typing fast in address bar and pressing enter leads to missing end characters. a=paolo
browser/base/content/test/general/browser.ini
browser/base/content/test/general/browser_autocomplete_enter_race.js
browser/base/content/urlbarBindings.xml
toolkit/content/widgets/autocomplete.xml
--- a/browser/base/content/test/general/browser.ini
+++ b/browser/base/content/test/general/browser.ini
@@ -119,16 +119,17 @@ skip-if = e10s # Bug 1093153 - no about:
 [browser_action_keyword_override.js]
 [browser_action_searchengine.js]
 [browser_action_searchengine_alias.js]
 [browser_addKeywordSearch.js]
 [browser_search_favicon.js]
 [browser_alltabslistener.js]
 [browser_autocomplete_a11y_label.js]
 skip-if = e10s # Bug 1101993 - times out for unknown reasons when run in the dir (works on its own)
+[browser_autocomplete_enter_race.js]
 [browser_autocomplete_no_title.js]
 [browser_autocomplete_autoselect.js]
 [browser_autocomplete_oldschool_wrap.js]
 [browser_autocomplete_tag_star_visibility.js]
 [browser_backButtonFitts.js]
 skip-if = os != "win" || e10s # The Fitts Law back button is only supported on Windows (bug 571454) / e10s - Bug 1099154: test touches content (attempts to add an event listener directly to the contentWindow)
 [browser_beforeunload_duplicate_dialogs.js]
 skip-if = e10s # bug 967873 means permitUnload doesn't work in e10s mode
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/general/browser_autocomplete_enter_race.js
@@ -0,0 +1,28 @@
+add_task(function*() {
+  // This test is only relevant if UnifiedComplete is enabled.
+  Services.prefs.setBoolPref("browser.urlbar.unifiedcomplete", true);
+
+  registerCleanupFunction(() => {
+    PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId);
+    Services.prefs.clearUserPref("browser.urlbar.unifiedcomplete");
+  });
+
+  let itemId =
+    PlacesUtils.bookmarks.insertBookmark(PlacesUtils.unfiledBookmarksFolderId,
+                                         NetUtil.newURI("http://example.com/?q=%s"),
+                                         PlacesUtils.bookmarks.DEFAULT_INDEX,
+                                         "test");
+  PlacesUtils.bookmarks.setKeywordForBookmark(itemId, "keyword");
+
+  yield new Promise(resolve => waitForFocus(resolve, window));
+
+  yield promiseAutocompleteResultPopup("keyword bear");
+  gURLBar.focus();
+  EventUtils.synthesizeKey("d", {});
+  EventUtils.synthesizeKey("VK_RETURN", {});
+
+  yield promiseTabLoadEvent(gBrowser.selectedTab);
+  is(gBrowser.selectedTab.linkedBrowser.currentURI.spec,
+     "http://example.com/?q=beard",
+     "Latest typed characters should have been used");
+});
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -119,16 +119,18 @@
         this.inputField.removeEventListener("mousedown", this, false);
         this.inputField.removeEventListener("mousemove", this, false);
         this.inputField.removeEventListener("mouseout", this, false);
         this.inputField.removeEventListener("overflow", this, false);
         this.inputField.removeEventListener("underflow", this, false);
       ]]></destructor>
 
       <field name="_value">""</field>
+      <field name="gotResultForCurrentQuery">false</field>
+      <field name="handleEnterWhenGotResult">false</field>
 
       <!--
         onBeforeValueGet is called by the base-binding's .value getter.
         It can return an object with a "value" property, to override the
         return value of the getter.
       -->
       <method name="onBeforeValueGet">
         <body><![CDATA[
@@ -801,21 +803,58 @@
 
       <method name="onInput">
         <parameter name="aEvent"/>
         <body><![CDATA[
           if (!this.mIgnoreInput && this.mController.input == this) {
             this._value = this.inputField.value;
             gBrowser.userTypedValue = this.value;
             this.valueIsTyped = true;
+            this.gotResultForCurrentQuery = false;
             this.mController.handleText();
           }
           this.resetActionType();
         ]]></body>
       </method>
+
+      <method name="handleEnter">
+        <body><![CDATA[
+          // When UnifiedComplete is enabled, we need to ensure we're using
+          // a selected autocomplete result. A result should automatically be
+          // selected by default (UnifiedComplete guarantees at least one
+          // result), however autocomplete is async and therefore we may not
+          // have a result set relating to the current input yet. If that
+          // happens, we need to mark that when the first result does get added,
+          // it needs to be handled as if enter was pressed with that first
+          // result selected.
+          // With UnifiedComplete disabled we don't have this problem, as the
+          // default is to use the value directly from the input field (without
+          // relying on autocomplete).
+
+          if (!Services.prefs.getBoolPref("browser.urlbar.unifiedcomplete")) {
+            return this.mController.handleEnter(false);
+          }
+
+          // If anything other than the default (first) result is selected, then
+          // it must have been manually selected by the human. We let this
+          // explicit choice be used, even if it may be related to a previous
+          // input.
+          // However, if the default result is automatically selected, we
+          // ensure that it corresponds to the current input.
+
+          if (this.popup.selectedIndex != 0 || this.gotResultForCurrentQuery) {
+            return this.mController.handleEnter(false);
+          }
+
+          this.handleEnterWhenGotResult = true;
+
+          return true;
+        ]]></body>
+      </method>
+
     </implementation>
 
     <handlers>
       <handler event="keydown"><![CDATA[
         if ((event.keyCode === KeyEvent.DOM_VK_ALT ||
              event.keyCode === KeyEvent.DOM_VK_SHIFT) &&
             this.popup.selectedIndex >= 0 &&
             !this._noActionsKeys.has(event.keyCode)) {
@@ -1476,16 +1515,22 @@
       <method name="onResultsAdded">
         <body>
           <![CDATA[
             if (!Services.prefs.getBoolPref("browser.urlbar.unifiedcomplete"))
               return;
 
             if (this._matchCount > 0 && this.selectedIndex == -1)
               this.selectedIndex = 0;
+
+            this.input.gotResultForCurrentQuery = true;
+            if (this.input.handleEnterWhenGotResult) {
+              this.input.handleEnterWhenGotResult = false;
+              this.input.mController.handleEnter(false);
+            }
           ]]>
         </body>
       </method>
 
     </implementation>
   </binding>
 
   <binding id="addon-progress-notification" extends="chrome://global/content/bindings/notification.xml#popup-notification">
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -125,17 +125,17 @@
 
       <property name="showImageColumn"
                 onset="this.setAttribute('showimagecolumn', val); return val;"
                 onget="return this.getAttribute('showimagecolumn') == 'true';"/>
 
       <property name="timeout"
                 onset="this.setAttribute('timeout', val); return val;">
         <getter><![CDATA[
-          // For security reasons delay searches on pasted values. 
+          // For security reasons delay searches on pasted values.
           if (this._valueIsPasted) {
             let t = parseInt(this.getAttribute('pastetimeout'));
             return isNaN(t) ? 1000 : t;
           }
 
           let t = parseInt(this.getAttribute('timeout'));
           return isNaN(t) ? 50 : t;
         ]]></getter>
@@ -473,17 +473,17 @@
 #endif
               this.mEnterEvent = aEvent;
               if (this.mController.selection) {
                 this._selectionDetails = {
                   index: this.mController.selection.currentIndex,
                   kind: "key"
                 };
               }
-              cancel = this.mController.handleEnter(false);
+              cancel = this.handleEnter();
               break;
             case KeyEvent.DOM_VK_DELETE:
 #ifdef XP_MACOSX
             case KeyEvent.DOM_VK_BACK_SPACE:
               if (aEvent.shiftKey)
 #endif
               cancel = this.mController.handleDelete();
               break;
@@ -503,16 +503,22 @@
             aEvent.stopPropagation();
             aEvent.preventDefault();
           }
 
           return true;
         ]]></body>
       </method>
 
+      <method name="handleEnter">
+        <body><![CDATA[
+          return this.mController.handleEnter(false);
+        ]]></body>
+      </method>
+
       <!-- ::::::::::::: miscellaneous ::::::::::::: -->
 
       <method name="initSearchNames">
         <body><![CDATA[
           if (!this.mSearchNames) {
             var names = this.getAttribute("autocompletesearch");
             if (!names)
               this.mSearchNames = [];