Bug 1292310 - Enter may wrongly confirm a mouse selected entry in the urlbar. r=adw
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 06 Sep 2016 17:55:56 +0200
changeset 313195 c35d8f056f6745e084a383ad3896afb8fd2facaa
parent 313194 3fe5546bc877aa00eeb88dcf4adb38e12f82f14d
child 313196 3d1777fa2c3435a38d02541ae5be76334de89bfb
push id30674
push userkwierso@gmail.com
push dateThu, 08 Sep 2016 22:03:48 +0000
treeherdermozilla-central@5d854c8d0765 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1292310
milestone51.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 1292310 - Enter may wrongly confirm a mouse selected entry in the urlbar. r=adw MozReview-Commit-ID: 6teMoPr1vb6
toolkit/components/autocomplete/nsAutoCompleteController.cpp
toolkit/components/autocomplete/tests/unit/test_finalCompleteValue_defaultIndex.js
--- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -1385,36 +1385,38 @@ nsAutoCompleteController::EnterMatch(boo
     bool shouldComplete;
     input->GetCompleteDefaultIndex(&shouldComplete);
     bool completeSelection;
     input->GetCompleteSelectedIndex(&completeSelection);
 
     int32_t selectedIndex;
     popup->GetSelectedIndex(&selectedIndex);
     if (selectedIndex >= 0) {
-
       nsAutoString inputValue;
       input->GetTextValue(inputValue);
-      nsAutoString finalValue;
-      if (!completeSelection || aIsPopupSelection ||
-          (mDefaultIndexCompleted &&
-           inputValue.Equals(mPlaceholderCompletionString,
-                             nsCaseInsensitiveStringComparator()))) {
+      bool defaultCompleted = mDefaultIndexCompleted &&
+                              inputValue.Equals(mPlaceholderCompletionString,
+                                                nsCaseInsensitiveStringComparator());
+      if (aIsPopupSelection || (!completeSelection && !defaultCompleted)) {
         // We need to fill-in the value if:
-        //  * completeselectedindex is false
+        //  * completeselectedindex is false and we didn't defaultComplete
         //  * A row in the popup was confirmed
-        //  * The default index completion was confirmed
-        GetResultValueAt(selectedIndex, true, finalValue);
-        value = finalValue;
+        GetResultValueAt(selectedIndex, true, value);
+      } else if (defaultCompleted) {
+        // We also need to fill-in the value if the default index completion was
+        // confirmed, though we cannot use the selectedIndex cause the selection
+        // may have been changed by the mouse in the meanwhile.
+        GetFinalDefaultCompleteValue(value);
       } else if (mCompletedSelectionIndex != -1) {
         // If completeselectedindex is true, and EnterMatch was not invoked by
         // mouse-clicking a match (for example the user pressed Enter),
         // don't fill in the value as it will have already been filled in as
         // needed, unless the selected match has a final complete value that
         // differs from the user-facing value.
+        nsAutoString finalValue;
         GetResultValueAt(mCompletedSelectionIndex, true, finalValue);
         nsAutoString completedValue;
         GetResultValueAt(mCompletedSelectionIndex, false, completedValue);
         if (completedValue.Equals(inputValue) && !finalValue.Equals(inputValue)) {
           value = finalValue;
         }
         // Note that if the user opens the popup, mouses over entries without
         // ever selecting one with the keyboard, and then hits enter, none of
--- a/toolkit/components/autocomplete/tests/unit/test_finalCompleteValue_defaultIndex.js
+++ b/toolkit/components/autocomplete/tests/unit/test_finalCompleteValue_defaultIndex.js
@@ -13,39 +13,63 @@ function AutoCompleteInput(aSearches) {
 }
 AutoCompleteInput.prototype = Object.create(AutoCompleteInputBase.prototype);
 
 add_test(function test_handleEnter() {
   let results = [
     ["mozilla.com", "https://www.mozilla.com"],
     ["gomozilla.org", "http://www.gomozilla.org"],
   ];
-  doSearch("moz", results, controller => {
+  doSearch("moz", results, 0, controller => {
     let input = controller.input;
     Assert.equal(input.textValue, "mozilla.com");
     Assert.equal(controller.getFinalCompleteValueAt(0), results[0][1]);
     Assert.equal(controller.getFinalCompleteValueAt(1), results[1][1]);
     Assert.equal(input.popup.selectedIndex, 0);
 
     controller.handleEnter(false);
     // Verify that the keyboard-selected thing got inserted,
     // and not the mouse selection:
     Assert.equal(controller.input.textValue, "https://www.mozilla.com");
   });
 });
 
-function doSearch(aSearchString, aResults, aOnCompleteCallback) {
+add_test(function test_handleEnter_otherSelected() {
+  // The popup selection may not coincide with what is filled into the input
+  // field, for example if the user changed it with the mouse and then pressed
+  // Enter. In such a case we should still use the inputField value and not the
+  // popup selected value.
+  let results = [
+    ["mozilla.com", "https://www.mozilla.com"],
+    ["gomozilla.org", "http://www.gomozilla.org"],
+  ];
+  doSearch("moz", results, 1, controller => {
+    let input = controller.input;
+    Assert.equal(input.textValue, "mozilla.com");
+    Assert.equal(controller.getFinalCompleteValueAt(0), results[0][1]);
+    Assert.equal(controller.getFinalCompleteValueAt(1), results[1][1]);
+    Assert.equal(input.popup.selectedIndex, 1);
+
+    controller.handleEnter(false);
+    // Verify that the keyboard-selected thing got inserted,
+    // and not the mouse selection:
+    Assert.equal(controller.input.textValue, "https://www.mozilla.com");
+  });
+});
+
+function doSearch(aSearchString, aResults, aSelectedIndex, aOnCompleteCallback) {
   let search = new AutoCompleteSearchBase(
     "search",
     new AutoCompleteResult(aResults)
   );
   registerAutoCompleteSearch(search);
 
   let input = new AutoCompleteInput([ search.name ]);
   input.textValue = aSearchString;
+  input.popup.selectedIndex = aSelectedIndex;
   // Needed for defaultIndex completion.
   input.selectTextRange(aSearchString.length, aSearchString.length);
 
   let controller = Cc["@mozilla.org/autocomplete/controller;1"].
                    getService(Ci.nsIAutoCompleteController);
   controller.input = input;
   controller.startSearch(aSearchString);