diff --git a/browser/components/search/content/search-one-offs.js b/browser/components/search/content/search-one-offs.js index fe6dda76127a..a7aff9358141 100644 --- a/browser/components/search/content/search-one-offs.js +++ b/browser/components/search/content/search-one-offs.js @@ -807,11 +807,11 @@ class SearchOneOffs { * to pass anything for this parameter. (Pass undefined or null.) * @returns {boolean} True if the one-offs handled the key press. */ - handleKeyDown(event, numListItems, allowEmptySelection, textboxUserValue) { + handleKeyPress(event, numListItems, allowEmptySelection, textboxUserValue) { if (!this.popup && !this._view) { return false; } - let handled = this._handleKeyDown( + let handled = this._handleKeyPress( event, numListItems, allowEmptySelection, @@ -824,7 +824,7 @@ class SearchOneOffs { return handled; } - _handleKeyDown(event, numListItems, allowEmptySelection, textboxUserValue) { + _handleKeyPress(event, numListItems, allowEmptySelection, textboxUserValue) { if (this.compact && this.container.hidden) { return false; } diff --git a/browser/components/search/content/searchbar.js b/browser/components/search/content/searchbar.js index 3fea7a59da49..c2099887ac00 100644 --- a/browser/components/search/content/searchbar.js +++ b/browser/components/search/content/searchbar.js @@ -574,40 +574,74 @@ this.textbox.popup.removeAttribute("showonlysettings"); }); - this.textbox.addEventListener("keydown", event => { - if ( - event.keyCode == KeyEvent.DOM_VK_UP && - event.getModifierState("Accel") - ) { - this.selectEngine(event, false); - } - }); + this.textbox.addEventListener( + "keypress", + event => { + // accel + up/down changes the default engine and shouldn't affect + // the selection on the one-off buttons. + let popup = this.textbox.popup; + if (!popup.popupOpen || event.getModifierState("Accel")) { + return; + } - this.textbox.addEventListener("keydown", event => { - if ( - event.keyCode == KeyEvent.DOM_VK_DOWN && - event.getModifierState("Accel") - ) { - this.selectEngine(event, true); - } - }); + let suggestionsHidden = + popup.richlistbox.getAttribute("collapsed") == "true"; + let numItems = suggestionsHidden ? 0 : popup.matchCount; + popup.oneOffButtons.handleKeyPress(event, numItems, true); + }, + true + ); - this.textbox.addEventListener("keydown", event => { - if ( - event.getModifierState("Alt") && - (event.keyCode == KeyEvent.DOM_VK_DOWN || - event.keyCode == KeyEvent.DOM_VK_UP) - ) { - this.textbox.openSearch(); - } - }); + this.textbox.addEventListener( + "keypress", + event => { + if ( + event.keyCode == KeyEvent.DOM_VK_UP && + event.getModifierState("Accel") + ) { + this.selectEngine(event, false); + } + }, + true + ); - if (AppConstants.platform == "macosx") { - this.textbox.addEventListener("keydown", event => { - if (event.keyCode == KeyEvent.DOM_VK_F4) { + this.textbox.addEventListener( + "keypress", + event => { + if ( + event.keyCode == KeyEvent.DOM_VK_DOWN && + event.getModifierState("Accel") + ) { + this.selectEngine(event, true); + } + }, + true + ); + + this.textbox.addEventListener( + "keypress", + event => { + if ( + event.getModifierState("Alt") && + (event.keyCode == KeyEvent.DOM_VK_DOWN || + event.keyCode == KeyEvent.DOM_VK_UP) + ) { this.textbox.openSearch(); } - }); + }, + true + ); + + if (AppConstants.platform == "macosx") { + this.textbox.addEventListener( + "keypress", + event => { + if (event.keyCode == KeyEvent.DOM_VK_F4) { + this.textbox.openSearch(); + } + }, + true + ); } this.textbox.addEventListener("dragover", event => { @@ -707,18 +741,6 @@ return aValue; }; - this.textbox.onBeforeHandleKeyDown = aEvent => { - let popup = this.textbox.popup; - if (!popup.popupOpen || aEvent.getModifierState("Accel")) { - return false; - } - - let suggestionsHidden = - popup.richlistbox.getAttribute("collapsed") == "true"; - let numItems = suggestionsHidden ? 0 : popup.matchCount; - return popup.oneOffButtons.handleKeyDown(aEvent, numItems, true); - }; - // This method overrides the autocomplete binding's openPopup (essentially // duplicating the logic from the autocomplete popup binding's // openAutocompletePopup method), modifying it so that the popup is aligned with diff --git a/browser/components/urlbar/UrlbarController.jsm b/browser/components/urlbar/UrlbarController.jsm index acc4c4de83d2..02217daa607c 100644 --- a/browser/components/urlbar/UrlbarController.jsm +++ b/browser/components/urlbar/UrlbarController.jsm @@ -285,7 +285,7 @@ class UrlbarController { if (this.view.isOpen && executeAction && this._lastQueryContextWrapper) { let { queryContext } = this._lastQueryContextWrapper; - let handled = this.view.oneOffSearchButtons.handleKeyDown( + let handled = this.view.oneOffSearchButtons.handleKeyPress( event, this.view.visibleElementCount, this.view.allowEmptySelection, diff --git a/toolkit/components/autocomplete/nsAutoCompleteController.cpp b/toolkit/components/autocomplete/nsAutoCompleteController.cpp index 1e27d48aa995..bb326127077a 100644 --- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp +++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp @@ -427,9 +427,13 @@ nsAutoCompleteController::HandleKeyNavigation(uint32_t aKey, bool* _retval) { // the cursor to home/end on some systems *_retval = true; bool reverse = aKey == dom::KeyboardEvent_Binding::DOM_VK_UP || - aKey == dom::KeyboardEvent_Binding::DOM_VK_PAGE_UP; + aKey == dom::KeyboardEvent_Binding::DOM_VK_PAGE_UP + ? true + : false; bool page = aKey == dom::KeyboardEvent_Binding::DOM_VK_PAGE_UP || - aKey == dom::KeyboardEvent_Binding::DOM_VK_PAGE_DOWN; + aKey == dom::KeyboardEvent_Binding::DOM_VK_PAGE_DOWN + ? true + : false; // Fill in the value of the textbox with whatever is selected in the popup // if the completeSelectedIndex attribute is set. We check this before diff --git a/toolkit/components/satchel/nsFormFillController.cpp b/toolkit/components/satchel/nsFormFillController.cpp index f4da6976836a..6a3f6e27c096 100644 --- a/toolkit/components/satchel/nsFormFillController.cpp +++ b/toolkit/components/satchel/nsFormFillController.cpp @@ -823,6 +823,8 @@ nsFormFillController::HandleFormEvent(Event* aEvent) { return MouseDown(aEvent); case eKeyDown: return KeyDown(aEvent); + case eKeyPress: + return KeyPress(aEvent); case eEditorInput: { nsCOMPtr input = do_QueryInterface(aEvent->GetComposedTarget()); if (!IsTextControl(input)) { @@ -1017,8 +1019,6 @@ nsresult nsFormFillController::KeyDown(Event* aEvent) { } bool cancel = false; - bool unused = false; - uint32_t k = keyEvent->KeyCode(); switch (k) { case KeyboardEvent_Binding::DOM_VK_RETURN: { @@ -1026,6 +1026,40 @@ nsresult nsFormFillController::KeyDown(Event* aEvent) { controller->HandleEnter(false, aEvent, &cancel); break; } + } + + if (cancel) { + aEvent->PreventDefault(); + // Don't let the page see the RETURN event when the popup is open + // (indicated by cancel=true) so sites don't manually submit forms + // (e.g. via submit.click()) without the autocompleted value being filled. + // Bug 286933 will fix this for other key events. + if (k == KeyboardEvent_Binding::DOM_VK_RETURN) { + aEvent->StopPropagation(); + } + } + return NS_OK; +} + +nsresult nsFormFillController::KeyPress(Event* aEvent) { + NS_ASSERTION(mController, "should have a controller!"); + + mPasswordPopupAutomaticallyOpened = false; + + if (!IsFocusedInputControlled()) { + return NS_OK; + } + + RefPtr keyEvent = aEvent->AsKeyboardEvent(); + if (!keyEvent) { + return NS_ERROR_FAILURE; + } + + bool cancel = false; + bool unused = false; + + uint32_t k = keyEvent->KeyCode(); + switch (k) { case KeyboardEvent_Binding::DOM_VK_DELETE: #ifndef XP_MACOSX { @@ -1107,13 +1141,6 @@ nsresult nsFormFillController::KeyDown(Event* aEvent) { if (cancel) { aEvent->PreventDefault(); - // Don't let the page see the RETURN event when the popup is open - // (indicated by cancel=true) so sites don't manually submit forms - // (e.g. via submit.click()) without the autocompleted value being filled. - // Bug 286933 will fix this for other key events. - if (k == KeyboardEvent_Binding::DOM_VK_RETURN) { - aEvent->StopPropagation(); - } } return NS_OK; diff --git a/toolkit/components/satchel/nsFormFillController.h b/toolkit/components/satchel/nsFormFillController.h index 6484d1b7fadb..e704d2877fa4 100644 --- a/toolkit/components/satchel/nsFormFillController.h +++ b/toolkit/components/satchel/nsFormFillController.h @@ -22,6 +22,11 @@ #include "nsCycleCollectionParticipant.h" #include "nsILoginReputation.h" +// X.h defines KeyPress +#ifdef KeyPress +# undef KeyPress +#endif + class nsFormHistory; class nsINode; class nsPIDOMWindowOuter; @@ -50,6 +55,7 @@ class nsFormFillController final : public nsIFormFillController, MOZ_CAN_RUN_SCRIPT nsresult Focus(mozilla::dom::Event* aEvent); MOZ_CAN_RUN_SCRIPT nsresult KeyDown(mozilla::dom::Event* aKeyEvent); + MOZ_CAN_RUN_SCRIPT nsresult KeyPress(mozilla::dom::Event* aKeyEvent); MOZ_CAN_RUN_SCRIPT nsresult MouseDown(mozilla::dom::Event* aMouseEvent); nsFormFillController(); @@ -60,6 +66,9 @@ class nsFormFillController final : public nsIFormFillController, void AddWindowListeners(nsPIDOMWindowOuter* aWindow); MOZ_CAN_RUN_SCRIPT void RemoveWindowListeners(nsPIDOMWindowOuter* aWindow); + void AddKeyListener(nsINode* aInput); + void RemoveKeyListener(); + MOZ_CAN_RUN_SCRIPT void StartControllingInput(mozilla::dom::HTMLInputElement* aInput); MOZ_CAN_RUN_SCRIPT void StopControllingInput(); diff --git a/toolkit/content/customElements.js b/toolkit/content/customElements.js index 94159d354771..8a2cdcad8a90 100644 --- a/toolkit/content/customElements.js +++ b/toolkit/content/customElements.js @@ -835,6 +835,17 @@ "chrome://global/content/elements/toolbarbutton.js", "chrome://global/content/elements/tree.js", "chrome://global/content/elements/wizard.js", + // We load autocomplete-input eagerly, because we use system event + // listeners to be able to see keypress events for non-printable keys + // (see bug 1434837), and in the capture phase, for some reason. + // + // That means that they can run after the actual internal element + // event listeners if custom element construction is not synchronous, and + // thus be default-prevented by them. + // + // This all seems pretty fishy, ideally we'd stop using keypress for + // these events, but for now this preserves behavior. + "chrome://global/content/elements/autocomplete-input.js", ]) { Services.scriptloader.loadSubScript(script, window); } @@ -848,10 +859,6 @@ "printpreview-toolbar", "chrome://global/content/printPreviewToolbar.js", ], - [ - "autocomplete-input", - "chrome://global/content/elements/autocomplete-input.js", - ], ["editor", "chrome://global/content/elements/editor.js"], ]) { customElements.setElementCreationCallback(tag, () => { diff --git a/toolkit/content/widgets/autocomplete-input.js b/toolkit/content/widgets/autocomplete-input.js index ec2a2f0a5cee..4a2898be7e4b 100644 --- a/toolkit/content/widgets/autocomplete-input.js +++ b/toolkit/content/widgets/autocomplete-input.js @@ -35,7 +35,10 @@ this.onInput(event); }); - this.addEventListener("keydown", event => this.handleKeyDown(event)); + this.addEventListener("keypress", event => this.handleKeyPress(event), { + capture: true, + mozSystemGroup: true, + }); this.addEventListener( "compositionstart", @@ -476,19 +479,12 @@ } } - handleKeyDown(aEvent) { + handleKeyPress(aEvent) { // Re: urlbarDeferred, see the comment in urlbarBindings.xml. if (aEvent.defaultPrevented && !aEvent.urlbarDeferred) { return false; } - if ( - typeof this.onBeforeHandleKeyDown == "function" && - this.onBeforeHandleKeyDown(aEvent) - ) { - return true; - } - const isMac = AppConstants.platform == "macosx"; var cancel = false;