Bug 1330111 - Don't open username autocomplete upon focus if a contextmenu is opening. r=johannh,daleharvey draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 07 Feb 2017 08:41:02 +0800
changeset 479584 bb4200281706bc5bdc7824eb9a9f0db15be2fbaf
parent 478923 2ab480cdfdfdc7fea7e329e22b9540e1918c4594
child 479585 da4b9e070943bff6af98d31797756f3ff5195143
push id44309
push usermozilla@noorenberghe.ca
push dateTue, 07 Feb 2017 00:41:56 +0000
reviewersjohannh, daleharvey
bugs1330111
milestone54.0a1
Bug 1330111 - Don't open username autocomplete upon focus if a contextmenu is opening. r=johannh,daleharvey The `focus` event is received before the `contextmenu` event. MozReview-Commit-ID: 4Ya0uXKXWC6
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/test/browser/browser.ini
toolkit/components/passwordmgr/test/browser/browser_context_menu_autocomplete_interaction.js
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -5,23 +5,25 @@
 "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 = 250;
 
 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");
 
 XPCOMUtils.defineLazyModuleGetter(this, "DeferredTask", "resource://gre/modules/DeferredTask.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FormLikeFactory",
                                   "resource://gre/modules/FormLikeFactory.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LoginRecipesContent",
                                   "resource://gre/modules/LoginRecipes.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "LoginHelper",
                                   "resource://gre/modules/LoginHelper.jsm");
@@ -34,16 +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 = 0;
 
 var observer = {
   QueryInterface : XPCOMUtils.generateQI([Ci.nsIObserver,
                                           Ci.nsIFormSubmitObserver,
                                           Ci.nsIWebProgressListener,
                                           Ci.nsIDOMEventListener,
                                           Ci.nsISupportsWeakReference]),
 
@@ -120,16 +123,25 @@ var observer = {
     }
 
     switch (aEvent.type) {
       // Only used for username fields.
       case "focus": {
         LoginManagerContent._onUsernameFocus(aEvent);
         break;
       }
+
+      case "contextmenu": {
+        gLastContextMenuEventTimeStamp = aEvent.timeStamp;
+        break;
+      }
+
+      default: {
+        throw new Error("Unexpected event");
+      }
     }
   },
 };
 
 Services.obs.addObserver(observer, "earlyformsubmit", false);
 var prefBranch = Services.prefs.getBranch("signon.");
 prefBranch.addObserver("", observer.onPrefChange, false);
 
@@ -553,23 +565,40 @@ var LoginManagerContent = {
       return;
     }
 
     if (this._isLoginAlreadyFilled(focusedField)) {
       log("_onUsernameFocus: Already filled");
       return;
     }
 
-    let formFillFocused = this._formFillService.focusedInput;
-    if (formFillFocused == focusedField) {
-      log("_onUsernameFocus: Opening the autocomplete popup");
-      this._formFillService.showPopup();
-    } else {
-      log("_onUsernameFocus: FormFillController has a different focused input");
-    }
+    /*
+     * A `focus` event is fired before a `contextmenu` event if a 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.
+     */
+    setTimeout(function maybeOpenAutocompleteAfterFocus() {
+      // Even though the `focus` event happens first, its .timeStamp is greater in
+      // testing and I don't want to rely on that so the absolute value is used.
+      let timeDiff = Math.abs(gLastContextMenuEventTimeStamp - event.timeStamp);
+      if (timeDiff < AUTOCOMPLETE_AFTER_CONTEXTMENU_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);
   },
 
   /**
    * Listens for DOMAutoComplete and blur events on an input field.
    */
   onUsernameInput(event) {
     if (!event.isTrusted)
       return;
@@ -1218,17 +1247,19 @@ var LoginManagerContent = {
               autofillResult !== AUTOFILL_RESULT.FILLED) {
             log("_fillForm: Opening username autocomplete popup since the form wasn't autofilled");
             this._formFillService.showPopup();
           }
         }
       }
 
       if (usernameField) {
+        log("_fillForm: Attaching event listeners to usernameField");
         usernameField.addEventListener("focus", observer);
+        usernameField.addEventListener("contextmenu", 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
@@ -33,16 +33,17 @@ support-files =
 [browser_capture_doorhanger_httpsUpgrade.js]
 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
+[browser_context_menu_autocomplete_interaction.js]
 [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]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/passwordmgr/test/browser/browser_context_menu_autocomplete_interaction.js
@@ -0,0 +1,99 @@
+/*
+ * Test the password manager context menu interaction with autocomplete.
+ */
+
+"use strict";
+
+const TEST_HOSTNAME = "https://example.com";
+const BASIC_FORM_PAGE_PATH = DIRECTORY_PATH + "form_basic.html";
+
+var gUnexpectedIsTODO = false;
+
+/**
+ * Initialize logins needed for the tests and disable autofill
+ * for login forms for easier testing of manual fill.
+ */
+add_task(function* test_initialize() {
+  let autocompletePopup = document.getElementById("PopupAutoComplete");
+  Services.prefs.setBoolPref("signon.autofillForms", false);
+  registerCleanupFunction(() => {
+    Services.prefs.clearUserPref("signon.autofillForms");
+    autocompletePopup.removeEventListener("popupshowing", autocompleteUnexpectedPopupShowing);
+  });
+  for (let login of loginList()) {
+    Services.logins.addLogin(login);
+  }
+  autocompletePopup.addEventListener("popupshowing", autocompleteUnexpectedPopupShowing);
+});
+
+add_task(function* test_context_menu_username() {
+  yield BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: TEST_HOSTNAME + BASIC_FORM_PAGE_PATH,
+  }, function* (browser) {
+    yield openContextMenu(browser, "#form-basic-username");
+
+    let contextMenu = document.getElementById("contentAreaContextMenu");
+    Assert.equal(contextMenu.state, "open", "Context menu opened");
+    contextMenu.hidePopup();
+  });
+});
+
+add_task(function* test_context_menu_password() {
+  gUnexpectedIsTODO = true;
+  yield BrowserTestUtils.withNewTab({
+    gBrowser,
+    url: TEST_HOSTNAME + BASIC_FORM_PAGE_PATH,
+  }, function* (browser) {
+    yield openContextMenu(browser, "#form-basic-password");
+
+    let contextMenu = document.getElementById("contentAreaContextMenu");
+    Assert.equal(contextMenu.state, "open", "Context menu opened");
+    contextMenu.hidePopup();
+  });
+});
+
+function autocompleteUnexpectedPopupShowing(event) {
+  if (gUnexpectedIsTODO) {
+    todo(false, "Autocomplete shouldn't appear");
+  } else {
+    Assert.ok(false, "Autocomplete shouldn't appear");
+  }
+  event.target.hidePopup();
+}
+
+/**
+ * Synthesize mouse clicks to open the context menu popup
+ * for a target login input element.
+ */
+function* openContextMenu(browser, loginInput) {
+  // First synthesize a mousedown. We need this to get the focus event with the "contextmenu" event.
+  let eventDetails1 = {type: "mousedown", button: 2};
+  BrowserTestUtils.synthesizeMouseAtCenter(loginInput, eventDetails1, browser);
+
+  // Then synthesize the contextmenu click over the input element.
+  let contextMenuShownPromise = BrowserTestUtils.waitForEvent(window, "popupshown");
+  let eventDetails = {type: "contextmenu", button: 2};
+  BrowserTestUtils.synthesizeMouseAtCenter(loginInput, eventDetails, browser);
+  yield contextMenuShownPromise;
+
+  // Wait to see which popups are shown.
+  yield new Promise(resolve => setTimeout(resolve, 1000));
+}
+
+function loginList() {
+  return [
+    LoginTestUtils.testData.formLogin({
+      hostname: "https://example.com",
+      formSubmitURL: "https://example.com",
+      username: "username",
+      password: "password",
+    }),
+    LoginTestUtils.testData.formLogin({
+      hostname: "https://example.com",
+      formSubmitURL: "https://example.com",
+      username: "username2",
+      password: "password2",
+    }),
+  ];
+}