Bug 1302472 - keyboard selected value in autocomplete popup cannot be confirmed with <enter> if completedefaultindex is true but completeselectedindex is false. r=adw
MozReview-Commit-ID: FR4pKGZLAl2
--- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -1391,25 +1391,33 @@ nsAutoCompleteController::EnterMatch(boo
bool completeSelection;
input->GetCompleteSelectedIndex(&completeSelection);
int32_t selectedIndex;
popup->GetSelectedIndex(&selectedIndex);
if (selectedIndex >= 0) {
nsAutoString inputValue;
input->GetTextValue(inputValue);
- bool defaultCompleted = mDefaultIndexCompleted &&
- inputValue.Equals(mPlaceholderCompletionString,
- nsCaseInsensitiveStringComparator());
- if (aIsPopupSelection || (!completeSelection && !defaultCompleted)) {
+ if (aIsPopupSelection || !completeSelection) {
// We need to fill-in the value if:
- // * completeselectedindex is false and we didn't defaultComplete
+ // * completeselectedindex is false
// * A row in the popup was confirmed
+ //
+ // TODO: This is not totally correct, cause it will also confirm
+ // a result selected with a simple mouseover, that could also have
+ // happened accidentally, maybe touching a touchpad.
+ // The reason is that autocomplete.xml sets selectedIndex on mousemove
+ // making impossible, in the !completeSelection case, to distinguish if
+ // the user wanted to confirm autoFill or the popup entry.
+ // The solution may be to change autocomplete.xml to set selectedIndex
+ // only on popupClick, but that requires changing the selection behavior.
GetResultValueAt(selectedIndex, true, value);
- } else if (defaultCompleted) {
+ } else if (mDefaultIndexCompleted &&
+ inputValue.Equals(mPlaceholderCompletionString,
+ nsCaseInsensitiveStringComparator())) {
// 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
--- a/toolkit/components/autocomplete/tests/unit/test_finalCompleteValue_defaultIndex.js
+++ b/toolkit/components/autocomplete/tests/unit/test_finalCompleteValue_defaultIndex.js
@@ -13,17 +13,17 @@ 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, 0, controller => {
+ doSearch("moz", results, { selectedIndex: 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,
@@ -36,40 +36,65 @@ add_test(function test_handleEnter_other
// 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 => {
+ doSearch("moz", results, { selectedIndex: 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) {
+add_test(function test_handleEnter_otherSelected_nocompleteselectedindex() {
+ let results = [
+ ["mozilla.com", "https://www.mozilla.com"],
+ ["gomozilla.org", "http://www.gomozilla.org"],
+ ];
+ doSearch("moz", results, { selectedIndex: 1,
+ completeSelectedIndex: false }, 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 result is inserted, not the
+ // defaultComplete.
+ Assert.equal(controller.input.textValue, "http://www.gomozilla.org");
+ });
+});
+
+function doSearch(aSearchString, aResults, aOptions, aOnCompleteCallback) {
let search = new AutoCompleteSearchBase(
"search",
new AutoCompleteResult(aResults)
);
registerAutoCompleteSearch(search);
let input = new AutoCompleteInput([ search.name ]);
input.textValue = aSearchString;
- input.popup.selectedIndex = aSelectedIndex;
+ if ("selectedIndex" in aOptions) {
+ input.popup.selectedIndex = aOptions.selectedIndex;
+ }
+ if ("completeSelectedIndex" in aOptions) {
+ input.completeSelectedIndex = aOptions.completeSelectedIndex;
+ }
// 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);