Bug 1337772 - Part 1 - Use mousedown instead of contextmenu to avoid showing the password autocomplete. r=MattN

MozReview-Commit-ID: EUZ1f6Qdm0c

--HG--
extra : rebase_source : e0b4f4f5d14407a831ca6efe4481f6c884682b39

toolkit/components/passwordmgr/LoginManagerContent.jsm

@@ -10,7 +10,7 @@ this.EXPORTED_SYMBOLS = [ "LoginManagerContent",
 
 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");
@@ -41,7 +41,7 @@ XPCOMUtils.defineLazyGetter(this, "log", () => {
 
 // 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,
@@ -129,10 +129,13 @@ var observer = {
         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;
       }
 
@@ -578,33 +581,24 @@ var LoginManagerContent = {
     }
 
     /*
-     * 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();
   },
 
   /**
@@ -1279,7 +1273,7 @@ var LoginManagerContent = {
       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);

toolkit/components/passwordmgr/test/browser/browser.ini

@@ -40,7 +40,6 @@ support-files =
   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

toolkit/components/satchel/nsFormFillController.cpp

@@ -41,7 +41,6 @@
 #include "nsIFrame.h"
 #include "nsIScriptSecurityManager.h"
 #include "nsFocusManager.h"
-#include "nsThreadUtils.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
@@ -73,11 +72,11 @@ nsFormFillController::nsFormFillController() :
   // 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),
@@ -957,9 +956,6 @@ nsFormFillController::HandleEvent(nsIDOMEvent* aEvent)
     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;
@@ -1043,33 +1039,6 @@ nsFormFillController::MaybeStartControllingInput(nsIDOMHTMLInputElement* 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)
 {
@@ -1086,8 +1055,28 @@ nsFormFillController::Focus(nsIDOMEvent* aEvent)
   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;
@@ -1219,6 +1208,15 @@ nsFormFillController::MouseDown(nsIDOMEvent* aEvent)
 
   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;
 

toolkit/components/satchel/nsFormFillController.h

@@ -30,7 +30,6 @@
 class nsFormHistory;
 class nsINode;
 class nsPIDOMWindowOuter;
-class nsIFormControl;
 
 class nsFormFillController final : public nsIFormFillController,
                                    public nsIAutoCompleteInput,
@@ -88,8 +87,6 @@ protected:
   void RemoveForDocument(nsIDocument* aDoc);
   bool IsEventTrusted(nsIDOMEvent *aEvent);
 
-  void FocusEventDelayedCallback(nsIFormControl* formControl);
-
   // members //////////////////////////////////////////
 
   nsCOMPtr<nsIAutoCompleteController> mController;
@@ -116,11 +113,11 @@ protected:
   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;