Bug 1472080 - Make Enter always autocomplete the selected result; r=bgrins.
This patch ensure that we have a consistent behavior when
the autocomplete popup is open and the user hit Enter.
We remove the autocompletePopupNavigated property which
is now useless.
We also make sure that we always show the popup when we
have at least one result (previously we needed to have
2 results to make it visible). This is again for
consistency sake and making it more obvious for the
user what will happen when hitting Enter or Tab.
Tests are modified to reflect these new behaviors.
MozReview-Commit-ID: 8QgoeU6DNk6
--- a/devtools/client/webconsole/components/JSTerm.js
+++ b/devtools/client/webconsole/components/JSTerm.js
@@ -124,24 +124,16 @@ class JSTerm extends Component {
this._lastFrameActorId = null;
/**
* Last input value.
* @type string
*/
this.lastInputValue = "";
- /**
- * Tells if the autocomplete popup was navigated since the last open.
- *
- * @private
- * @type boolean
- */
- this._autocompletePopupNavigated = false;
-
this.autocompletePopup = null;
this.inputNode = null;
this.completeNode = null;
this.COMPLETE_FORWARD = 0;
this.COMPLETE_BACKWARD = 1;
this.COMPLETE_HINT_ONLY = 2;
this.COMPLETE_PAGEUP = 3;
@@ -187,19 +179,19 @@ class JSTerm extends Component {
if (
!this.autocompletePopup.isOpen &&
!Debugger.isCompilableUnit(this.getInputValue())
) {
// incomplete statement
return "CodeMirror.Pass";
}
- if (this._autocompletePopupNavigated &&
- this.autocompletePopup.isOpen &&
- this.autocompletePopup.selectedIndex > -1
+ if (
+ this.autocompletePopup.isOpen
+ && this.autocompletePopup.selectedIndex > -1
) {
return this.acceptProposedCompletion();
}
this.execute();
return null;
},
@@ -226,36 +218,30 @@ class JSTerm extends Component {
// Input is not empty and some text is selected, let the editor handle this.
return true;
},
"Up": () => {
let inputUpdated;
if (this.autocompletePopup.isOpen) {
inputUpdated = this.complete(this.COMPLETE_BACKWARD);
- if (inputUpdated) {
- this._autocompletePopupNavigated = true;
- }
} else if (this.canCaretGoPrevious()) {
inputUpdated = this.historyPeruse(HISTORY_BACK);
}
if (!inputUpdated) {
return "CodeMirror.Pass";
}
return null;
},
"Down": () => {
let inputUpdated;
if (this.autocompletePopup.isOpen) {
inputUpdated = this.complete(this.COMPLETE_FORWARD);
- if (inputUpdated) {
- this._autocompletePopupNavigated = true;
- }
} else if (this.canCaretGoNext()) {
inputUpdated = this.historyPeruse(HISTORY_FORWARD);
}
if (!inputUpdated) {
return "CodeMirror.Pass";
}
return null;
@@ -266,22 +252,19 @@ class JSTerm extends Component {
this.clearCompletion();
}
return "CodeMirror.Pass";
},
"Right": () => {
const haveSuggestion =
this.autocompletePopup.isOpen || this.lastCompletion.value;
- const useCompletion =
- this.canCaretGoNext() || this._autocompletePopupNavigated;
if (
haveSuggestion &&
- useCompletion &&
this.complete(this.COMPLETE_HINT_ONLY) &&
this.lastCompletion.value &&
this.acceptProposedCompletion()
) {
return null;
}
if (this.autocompletePopup.isOpen) {
@@ -329,30 +312,26 @@ class JSTerm extends Component {
return null;
}
return "CodeMirror.Pass";
},
"PageUp": () => {
if (this.autocompletePopup.isOpen) {
- if (this.complete(this.COMPLETE_PAGEUP)) {
- this._autocompletePopupNavigated = true;
- }
+ this.complete(this.COMPLETE_PAGEUP);
return null;
}
return "CodeMirror.Pass";
},
"PageDown": () => {
if (this.autocompletePopup.isOpen) {
- if (this.complete(this.COMPLETE_PAGEDOWN)) {
- this._autocompletePopupNavigated = true;
- }
+ this.complete(this.COMPLETE_PAGEDOWN);
return null;
}
return "CodeMirror.Pass";
},
"Home": () => {
if (this.autocompletePopup.isOpen) {
@@ -840,76 +819,64 @@ class JSTerm extends Component {
if (this.autocompletePopup.isOpen) {
this.clearCompletion();
event.preventDefault();
event.stopPropagation();
}
break;
case KeyCodes.DOM_VK_RETURN:
- if (this._autocompletePopupNavigated &&
+ if (
this.autocompletePopup.isOpen &&
this.autocompletePopup.selectedIndex > -1) {
this.acceptProposedCompletion();
} else {
this.execute();
}
event.preventDefault();
break;
case KeyCodes.DOM_VK_UP:
if (this.autocompletePopup.isOpen) {
inputUpdated = this.complete(this.COMPLETE_BACKWARD);
- if (inputUpdated) {
- this._autocompletePopupNavigated = true;
- }
} else if (this.canCaretGoPrevious()) {
inputUpdated = this.historyPeruse(HISTORY_BACK);
}
if (inputUpdated) {
event.preventDefault();
}
break;
case KeyCodes.DOM_VK_DOWN:
if (this.autocompletePopup.isOpen) {
inputUpdated = this.complete(this.COMPLETE_FORWARD);
- if (inputUpdated) {
- this._autocompletePopupNavigated = true;
- }
} else if (this.canCaretGoNext()) {
inputUpdated = this.historyPeruse(HISTORY_FORWARD);
}
if (inputUpdated) {
event.preventDefault();
}
break;
case KeyCodes.DOM_VK_PAGE_UP:
if (this.autocompletePopup.isOpen) {
inputUpdated = this.complete(this.COMPLETE_PAGEUP);
- if (inputUpdated) {
- this._autocompletePopupNavigated = true;
- }
} else {
this.hud.outputScroller.scrollTop =
Math.max(0,
this.hud.outputScroller.scrollTop -
this.hud.outputScroller.clientHeight
);
}
event.preventDefault();
break;
case KeyCodes.DOM_VK_PAGE_DOWN:
if (this.autocompletePopup.isOpen) {
inputUpdated = this.complete(this.COMPLETE_PAGEDOWN);
- if (inputUpdated) {
- this._autocompletePopupNavigated = true;
- }
} else {
this.hud.outputScroller.scrollTop =
Math.min(this.hud.outputScroller.scrollHeight,
this.hud.outputScroller.scrollTop +
this.hud.outputScroller.clientHeight
);
}
event.preventDefault();
@@ -939,27 +906,23 @@ class JSTerm extends Component {
case KeyCodes.DOM_VK_LEFT:
if (this.autocompletePopup.isOpen || this.lastCompletion.value) {
this.clearCompletion();
}
break;
case KeyCodes.DOM_VK_RIGHT:
- const cursorAtTheEnd = this.inputNode.selectionStart ==
- this.inputNode.selectionEnd &&
- this.inputNode.selectionStart ==
- inputValue.length;
- const haveSuggestion = this.autocompletePopup.isOpen ||
- this.lastCompletion.value;
- const useCompletion = cursorAtTheEnd || this._autocompletePopupNavigated;
- if (haveSuggestion && useCompletion &&
- this.complete(this.COMPLETE_HINT_ONLY) &&
- this.lastCompletion.value &&
- this.acceptProposedCompletion()) {
+ const haveSuggestion = this.autocompletePopup.isOpen || this.lastCompletion.value;
+ if (
+ haveSuggestion &&
+ this.complete(this.COMPLETE_HINT_ONLY) &&
+ this.lastCompletion.value &&
+ this.acceptProposedCompletion()
+ ) {
event.preventDefault();
}
if (this.autocompletePopup.isOpen) {
this.clearCompletion();
}
break;
case KeyCodes.DOM_VK_TAB:
@@ -1296,17 +1259,17 @@ class JSTerm extends Component {
const popup = this.autocompletePopup;
popup.setItems(items);
const completionType = this.lastCompletion.completionType;
this.lastCompletion = {
value: inputValue,
matchProp: lastPart,
};
- if (items.length > 1 && !popup.isOpen) {
+ if (items.length > 0 && !popup.isOpen) {
let popupAlignElement;
let xOffset;
let yOffset;
if (this.editor) {
popupAlignElement = this.node.querySelector(".CodeMirror-cursor");
// We need to show the popup at the ".".
xOffset = -1 * lastPart.length * this._inputCharWidth;
@@ -1316,22 +1279,21 @@ class JSTerm extends Component {
(inputUntilCursor.lastIndexOf("\n") + 1) -
lastPart.length;
xOffset = (offset * this._inputCharWidth) + this._chevronWidth;
popupAlignElement = this.inputNode;
}
if (popupAlignElement) {
popup.openPopup(popupAlignElement, xOffset, yOffset);
- this._autocompletePopupNavigated = false;
}
- } else if (items.length < 2 && popup.isOpen) {
+ } else if (items.length === 0 && popup.isOpen) {
popup.hidePopup();
- this._autocompletePopupNavigated = false;
}
+
if (items.length == 1) {
popup.selectedIndex = 0;
}
this.onAutocompleteSelect();
if (completionType != this.COMPLETE_HINT_ONLY && popup.itemCount == 1) {
this.acceptProposedCompletion();
@@ -1376,17 +1338,16 @@ class JSTerm extends Component {
// value again.
if (this.inputNode) {
this.inputNode.blur();
}
this.autocompletePopup.once("popup-closed", () => {
this.focus();
});
this.autocompletePopup.hidePopup();
- this._autocompletePopupNavigated = false;
}
}
}
/**
* Accept the proposed input completion.
*
* @return boolean
--- a/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_return_key_no_selection.js
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_autocomplete_return_key_no_selection.js
@@ -45,20 +45,23 @@ async function performTests() {
await onPopUpOpen;
ok(popup.isOpen, "popup is open");
is(popup.itemCount, 2, "popup.itemCount is correct");
isnot(popup.selectedIndex, -1, "popup.selectedIndex is correct");
info("press Return and wait for popup to hide");
const onPopUpClose = popup.once("popup-closed");
- executeSoon(() => EventUtils.synthesizeKey("KEY_Enter"));
+ EventUtils.synthesizeKey("KEY_Enter");
await onPopUpClose;
ok(!popup.isOpen, "popup is not open after KEY_Enter");
- is(jsterm.getInputValue(), "", "inputNode is empty after KEY_Enter");
+ is(jsterm.getInputValue(), "window.testBugA",
+ "input was completed with the first item of the popup");
ok(!getJsTermCompletionValue(jsterm), "completeNode is empty");
+ EventUtils.synthesizeKey("KEY_Enter");
+ is(jsterm.getInputValue(), "", "input is empty after KEY_Enter");
+
const state = ui.consoleOutput.getStore().getState();
const entries = getHistoryEntries(state);
- is(entries[entries.length - 1], "window.testBug",
- "jsterm history is correct");
+ is(entries[entries.length - 1], "window.testBugA", "jsterm history is correct");
}
--- a/devtools/client/webconsole/test/mochitest/browser_jsterm_completion.js
+++ b/devtools/client/webconsole/test/mochitest/browser_jsterm_completion.js
@@ -14,21 +14,24 @@ add_task(async function() {
await performTests();
// And then run it with the CodeMirror-powered one.
await pushPref("devtools.webconsole.jsterm.codeMirror", true);
await performTests();
});
async function performTests() {
const {jsterm, ui} = await openNewTabAndConsole(TEST_URI);
+ const {autocompletePopup} = jsterm;
// Test typing 'docu'.
await jstermSetValueAndComplete(jsterm, "docu");
is(jsterm.getInputValue(), "docu", "'docu' completion (input.value)");
checkJsTermCompletionValue(jsterm, " ment", "'docu' completion (completeNode)");
+ is(autocompletePopup.items.length, 1, "autocomplete popup has 1 item");
+ ok(autocompletePopup.isOpen, "autocomplete popup is open with 1 item");
// Test typing 'docu' and press tab.
await jstermSetValueAndComplete(jsterm, "docu", undefined, jsterm.COMPLETE_FORWARD);
is(jsterm.getInputValue(), "document", "'docu' tab completion");
checkJsTermCursor(jsterm, "document".length, "cursor is at the end of 'document'");
is(getJsTermCompletionValue(jsterm).replace(/ /g, ""), "", "'docu' completed");