Bug 1292310 - Enter may wrongly confirm a mouse selected entry in the urlbar. r=adw, a=ritu
authorMarco Bonardo <mbonardo@mozilla.com>
Tue, 06 Sep 2016 17:55:56 +0200
changeset 350192 fd095a122f65c56fd1198468adbf280e59c6cfa7
parent 350191 76e76e26bfe70e13bf6c91bfb2530f9acc42687a
child 350193 696a981b6d534a7015d6f2ba3d2379af620e9b26
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw, ritu
bugs1292310
milestone50.0a2
Bug 1292310 - Enter may wrongly confirm a mouse selected entry in the urlbar. r=adw, a=ritu 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
@@ -1373,36 +1373,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);