Bug 1330111 - Don't open username autocomplete upon focus if a contextmenu is opening. r=johannh,daleharvey
☠☠ backed out by b3243521586b ☠ ☠
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 07 Feb 2017 08:41:02 +0800
changeset 341058 9c3763aa4bed5036916ea0b9e6a4a0b331a7e24f
parent 341057 2dee1d713c1d562341174eadcd90c6be2a01ef8b
child 341059 d71bc4a09493d50b118b850d3f53b47bfdf9bd87
push id86621
push usermozilla@noorenberghe.ca
push dateTue, 07 Feb 2017 05:12:02 +0000
treeherdermozilla-inbound@d71bc4a09493 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh, daleharvey
bugs1330111
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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);
 
@@ -555,23 +567,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;
@@ -1220,17 +1249,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",
+    }),
+  ];
+}