Bug 1337772 - Part 1 - Use mousedown instead of contextmenu to avoid showing the password autocomplete. r=MattN
MozReview-Commit-ID: EUZ1f6Qdm0c
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -5,17 +5,17 @@
"use strict";
this.EXPORTED_SYMBOLS = [ "LoginManagerContent",
"LoginFormFactory",
"UserAutoCompleteResult" ];
const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
const PASSWORD_INPUT_ADDED_COALESCING_THRESHOLD_MS = 1;
-const AUTOCOMPLETE_AFTER_CONTEXTMENU_THRESHOLD_MS = 400;
+const AUTOCOMPLETE_AFTER_RIGHT_CLICK_THRESHOLD_MS = 400;
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
Cu.import("resource://gre/modules/InsecurePasswordUtils.jsm");
Cu.import("resource://gre/modules/Promise.jsm");
Cu.import("resource://gre/modules/Preferences.jsm");
Cu.import("resource://gre/modules/Timer.jsm");
@@ -36,17 +36,17 @@ XPCOMUtils.defineLazyServiceGetter(this,
XPCOMUtils.defineLazyGetter(this, "log", () => {
let logger = LoginHelper.createLogger("LoginManagerContent");
return logger.log.bind(logger);
});
// These mirror signon.* prefs.
var gEnabled, gAutofillForms, gStoreWhenAutocompleteOff;
-var gLastContextMenuEventTimeStamp = Number.NEGATIVE_INFINITY;
+var gLastRightClickTimeStamp = Number.NEGATIVE_INFINITY;
var observer = {
QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
Ci.nsIFormSubmitObserver,
Ci.nsIWebProgressListener,
Ci.nsIDOMEventListener,
Ci.nsISupportsWeakReference]),
@@ -124,20 +124,23 @@ var observer = {
switch (aEvent.type) {
// Only used for username fields.
case "focus": {
LoginManagerContent._onUsernameFocus(aEvent);
break;
}
- case "contextmenu": {
- // Date.now() is used instead of event.timeStamp since
- // dom.event.highrestimestamp.enabled isn't true on all channels yet.
- gLastContextMenuEventTimeStamp = Date.now();
+ case "mousedown": {
+ if (aEvent.button == 2) {
+ // Date.now() is used instead of event.timeStamp since
+ // dom.event.highrestimestamp.enabled isn't true on all channels yet.
+ gLastRightClickTimeStamp = Date.now();
+ }
+
break;
}
default: {
throw new Error("Unexpected event");
}
}
},
@@ -570,43 +573,34 @@ var LoginManagerContent = {
}
if (this._isLoginAlreadyFilled(focusedField)) {
log("_onUsernameFocus: Already filled");
return;
}
/*
- * A `focus` event is fired before a `contextmenu` event if a user right-clicks into an
+ * A `mousedown` event is fired before the `focus` event if the user right clicks into an
* unfocused field. In that case we don't want to show both autocomplete and a context menu
- * overlapping so we spin the event loop to see if a `contextmenu` event is coming next. If no
- * `contextmenu` event was seen and the focused field is still focused by the form fill
- * controller then show the autocomplete popup.
+ * overlapping so we check against the timestamp that was set by the `mousedown` event if the
+ * button code indicated a right click.
+ * We use a timestamp instead of a bool to avoid complexity when dealing with multiple input
+ * forms and the fact that a mousedown into an already focused field does not trigger another focus.
* Date.now() is used instead of event.timeStamp since dom.event.highrestimestamp.enabled isn't
* true on all channels yet.
*/
- let timestamp = Date.now();
- setTimeout(function maybeOpenAutocompleteAfterFocus() {
- // Even though the `focus` event happens first in testing, I don't want to
- // rely on that since it was supposedly in the opposite order before. Use
- // the absolute value to handle both orders.
- let timeDiff = Math.abs(gLastContextMenuEventTimeStamp - timestamp);
- if (timeDiff < AUTOCOMPLETE_AFTER_CONTEXTMENU_THRESHOLD_MS) {
- log("Not opening autocomplete after focus since a context menu was opened within",
- timeDiff, "ms");
- return;
- }
+ let timeDiff = Date.now() - gLastRightClickTimeStamp;
+ if (timeDiff < AUTOCOMPLETE_AFTER_RIGHT_CLICK_THRESHOLD_MS) {
+ log("Not opening autocomplete after focus since a context menu was opened within",
+ timeDiff, "ms");
+ return;
+ }
- if (this._formFillService.focusedInput == focusedField) {
- log("maybeOpenAutocompleteAfterFocus: Opening the autocomplete popup");
- this._formFillService.showPopup();
- } else {
- log("maybeOpenAutocompleteAfterFocus: FormFillController has a different focused input");
- }
- }.bind(this), 0);
+ log("maybeOpenAutocompleteAfterFocus: Opening the autocomplete popup");
+ this._formFillService.showPopup();
},
/**
* Listens for DOMAutoComplete and blur events on an input field.
*/
onUsernameInput(event) {
if (!event.isTrusted)
return;
@@ -1257,17 +1251,17 @@ var LoginManagerContent = {
this._formFillService.showPopup();
}
}
}
if (usernameField) {
log("_fillForm: Attaching event listeners to usernameField");
usernameField.addEventListener("focus", observer);
- usernameField.addEventListener("contextmenu", observer);
+ usernameField.addEventListener("mousedown", observer);
}
Services.obs.notifyObservers(form.rootElement, "passwordmgr-processed-form", null);
}
},
/**
* Given a field, determine whether that field was last filled as a username
--- a/toolkit/components/passwordmgr/test/browser/browser.ini
+++ b/toolkit/components/passwordmgr/test/browser/browser.ini
@@ -35,17 +35,16 @@ support-files =
subtst_notifications_1.html
subtst_notifications_8.html
[browser_capture_doorhanger_window_open.js]
support-files =
subtst_notifications_11.html
subtst_notifications_11_popup.html
skip-if = os == "linux" # Bug 1312981, bug 1313136
[browser_context_menu_autocomplete_interaction.js]
-skip-if = asan || (os == 'linux') # disabled on asan and linux * (see bug 1337772)
[browser_username_select_dialog.js]
support-files =
subtst_notifications_change_p.html
[browser_DOMFormHasPassword.js]
[browser_DOMInputPasswordAdded.js]
[browser_exceptions_dialog.js]
[browser_formless_submit_chrome.js]
[browser_hasInsecureLoginForms.js]
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -36,17 +36,16 @@
#include "nsToolkitCompsCID.h"
#include "nsEmbedCID.h"
#include "nsIDOMNSEditableElement.h"
#include "nsContentUtils.h"
#include "nsILoadContext.h"
#include "nsIFrame.h"
#include "nsIScriptSecurityManager.h"
#include "nsFocusManager.h"
-#include "nsThreadUtils.h"
using namespace mozilla;
using namespace mozilla::dom;
using mozilla::ErrorResult;
NS_IMPL_CYCLE_COLLECTION(nsFormFillController,
mController, mLoginManager, mFocusedPopup, mDocShells,
mPopups, mLastListener, mLastFormAutoComplete)
@@ -68,21 +67,21 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(nsFormF
nsFormFillController::nsFormFillController() :
mFocusedInput(nullptr),
mFocusedInputNode(nullptr),
mListNode(nullptr),
// The amount of time a context menu event supresses showing a
// popup from a focus event in ms. This matches the threshold in
// toolkit/components/passwordmgr/LoginManagerContent.jsm.
- mFocusAfterContextMenuThreshold(400),
+ mFocusAfterRightClickThreshold(400),
mTimeout(50),
mMinResultsForPopup(1),
mMaxRows(0),
- mLastContextMenuEventTimeStamp(TimeStamp()),
+ mLastRightClickTimeStamp(TimeStamp()),
mDisableAutoComplete(false),
mCompleteDefaultIndex(false),
mCompleteSelectedIndex(false),
mForceComplete(false),
mSuppressOnInput(false)
{
mController = do_GetService("@mozilla.org/autocomplete/controller;1");
MOZ_ASSERT(mController);
@@ -952,19 +951,16 @@ nsFormFillController::HandleEvent(nsIDOM
}
if (type.EqualsLiteral("compositionend")) {
NS_ASSERTION(mController, "should have a controller!");
if (mController && mFocusedInput)
mController->HandleEndComposition();
return NS_OK;
}
if (type.EqualsLiteral("contextmenu")) {
- // Set timestamp to check for a recent contextmenu
- // call in Focus(), to avoid showing the popup.
- mLastContextMenuEventTimeStamp = TimeStamp::Now();
if (mFocusedPopup)
mFocusedPopup->ClosePopup();
return NS_OK;
}
if (type.EqualsLiteral("pagehide")) {
nsCOMPtr<nsIDocument> doc = do_QueryInterface(
aEvent->InternalDOMEvent()->GetTarget());
@@ -1038,61 +1034,54 @@ nsFormFillController::MaybeStartControll
isPwmgrInput = true;
}
if (isPwmgrInput || hasList || autocomplete) {
StartControllingInput(aInput);
}
}
-void
-nsFormFillController::FocusEventDelayedCallback(nsIFormControl* aFormControl)
-{
- nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(mFocusedInputNode);
-
- if (!formControl || formControl != aFormControl ||
- formControl->ControlType() != NS_FORM_INPUT_PASSWORD) {
- return;
- }
-
- // If we have not seen a context menu call yet, just show the popup.
- if (mLastContextMenuEventTimeStamp.IsNull()) {
- ShowPopup();
- return;
- }
-
- uint64_t timeDiff = fabs((TimeStamp::Now() - mLastContextMenuEventTimeStamp).ToMilliseconds());
- // If this focus doesn't follow a contextmenu event within our specified
- // threshold then show the autocomplete popup for all password fields.
- // This is done to avoid showing both the context menu and the popup
- // at the same time. The threshold should be a low amount of time that
- // makes it impossible for the user to accidentally trigger this condition.
- if (timeDiff > mFocusAfterContextMenuThreshold) {
- ShowPopup();
- }
-}
-
nsresult
nsFormFillController::Focus(nsIDOMEvent* aEvent)
{
nsCOMPtr<nsIDOMHTMLInputElement> input = do_QueryInterface(
aEvent->InternalDOMEvent()->GetTarget());
MaybeStartControllingInput(input);
// Bail if we didn't start controlling the input.
if (!mFocusedInputNode) {
return NS_OK;
}
#ifndef ANDROID
nsCOMPtr<nsIFormControl> formControl = do_QueryInterface(mFocusedInputNode);
MOZ_ASSERT(formControl);
- NS_DispatchToMainThread(NewRunnableMethod<nsCOMPtr<nsIFormControl>>(
- this, &nsFormFillController::FocusEventDelayedCallback, formControl));
+ // If this focus doesn't follow a right click within our specified
+ // threshold then show the autocomplete popup for all password fields.
+ // This is done to avoid showing both the context menu and the popup
+ // at the same time.
+ // We use a timestamp instead of a bool to avoid complexity when dealing with
+ // multiple input forms and the fact that a mousedown into an already focused
+ // field does not trigger another focus.
+
+ if (formControl->ControlType() != NS_FORM_INPUT_PASSWORD) {
+ return NS_OK;
+ }
+
+ // If we have not seen a right click yet, just show the popup.
+ if (mLastRightClickTimeStamp.IsNull()) {
+ ShowPopup();
+ return NS_OK;
+ }
+
+ uint64_t timeDiff = (TimeStamp::Now() - mLastRightClickTimeStamp).ToMilliseconds();
+ if (timeDiff > mFocusAfterRightClickThreshold) {
+ ShowPopup();
+ }
#endif
return NS_OK;
}
nsresult
nsFormFillController::KeyPress(nsIDOMEvent* aEvent)
{
@@ -1214,16 +1203,25 @@ nsFormFillController::MouseDown(nsIDOMEv
nsCOMPtr<nsIDOMHTMLInputElement> targetInput = do_QueryInterface(
aEvent->InternalDOMEvent()->GetTarget());
if (!targetInput)
return NS_OK;
int16_t button;
mouseEvent->GetButton(&button);
+
+ // In case of a right click we set a timestamp that
+ // will be checked in Focus() to avoid showing
+ // both contextmenu and popup at the same time.
+ if (button == 2) {
+ mLastRightClickTimeStamp = TimeStamp::Now();
+ return NS_OK;
+ }
+
if (button != 0)
return NS_OK;
return ShowPopup();
}
NS_IMETHODIMP
nsFormFillController::ShowPopup()
--- a/toolkit/components/satchel/nsFormFillController.h
+++ b/toolkit/components/satchel/nsFormFillController.h
@@ -25,17 +25,16 @@
// X.h defines KeyPress
#ifdef KeyPress
#undef KeyPress
#endif
class nsFormHistory;
class nsINode;
class nsPIDOMWindowOuter;
-class nsIFormControl;
class nsFormFillController final : public nsIFormFillController,
public nsIAutoCompleteInput,
public nsIAutoCompleteSearch,
public nsIDOMEventListener,
public nsIFormAutoCompleteObserver,
public nsIMutationObserver
{
@@ -83,18 +82,16 @@ protected:
inline nsPIDOMWindowOuter *GetWindowForDocShell(nsIDocShell *aDocShell);
inline int32_t GetIndexOfDocShell(nsIDocShell *aDocShell);
void MaybeRemoveMutationObserver(nsINode* aNode);
void RemoveForDocument(nsIDocument* aDoc);
bool IsEventTrusted(nsIDOMEvent *aEvent);
- void FocusEventDelayedCallback(nsIFormControl* formControl);
-
// members //////////////////////////////////////////
nsCOMPtr<nsIAutoCompleteController> mController;
nsCOMPtr<nsILoginManager> mLoginManager;
nsIDOMHTMLInputElement* mFocusedInput;
nsINode* mFocusedInputNode;
// mListNode is a <datalist> element which, is set, has the form fill controller
@@ -111,21 +108,21 @@ protected:
// This is cleared by StopSearch().
nsCOMPtr<nsIFormAutoComplete> mLastFormAutoComplete;
nsString mLastSearchString;
nsDataHashtable<nsPtrHashKey<const nsINode>, bool> mPwmgrInputs;
nsDataHashtable<nsPtrHashKey<const nsINode>, bool> mAutofillInputs;
- uint16_t mFocusAfterContextMenuThreshold;
+ uint16_t mFocusAfterRightClickThreshold;
uint32_t mTimeout;
uint32_t mMinResultsForPopup;
uint32_t mMaxRows;
- mozilla::TimeStamp mLastContextMenuEventTimeStamp;
+ mozilla::TimeStamp mLastRightClickTimeStamp;
bool mDisableAutoComplete;
bool mCompleteDefaultIndex;
bool mCompleteSelectedIndex;
bool mForceComplete;
bool mSuppressOnInput;
};
#endif // __nsFormFillController__