Bug 1226236 - Allow user to remove saved login/password when being asked to update it; r=MattN
authorBianca Danforth <bdanforth@mozilla.com>
Wed, 13 May 2020 16:41:48 +0000
changeset 529691 941aaea7104eb93ff0f767c7ced85152128f2590
parent 529690 454c02806bd2323e71e0d05ec5f892ab32caacb6
child 529692 298f90202eda7df9de5c918a218057ce973c4f72
push id37414
push usernbeleuzu@mozilla.com
push dateThu, 14 May 2020 02:40:10 +0000
treeherdermozilla-central@045d696faa87 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1226236
milestone78.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 1226236 - Allow user to remove saved login/password when being asked to update it; r=MattN Differential Revision: https://phabricator.services.mozilla.com/D73177
browser/base/content/browser.js
browser/locales/en-US/chrome/browser/browser.properties
browser/themes/shared/customizableui/panelUI.inc.css
toolkit/components/passwordmgr/LoginManagerPrompter.jsm
toolkit/components/passwordmgr/test/browser/browser_doorhanger_generated_password.js
toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js
toolkit/components/passwordmgr/test/browser/head.js
toolkit/components/telemetry/Histograms.json
toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -9096,16 +9096,18 @@ var ConfirmationHint = {
       this._description.hidden = true;
       this._panel.classList.remove("with-description");
     }
 
     if (options.hideArrow) {
       this._panel.setAttribute("hidearrow", "true");
     }
 
+    this._panel.setAttribute("data-message-id", messageId);
+
     // The timeout value used here allows the panel to stay open for
     // 1.5s second after the text transition (duration=120ms) has finished.
     // If there is a description, we show for 4s after the text transition.
     const DURATION = options.showDescription ? 4000 : 1500;
     this._panel.addEventListener(
       "popupshown",
       () => {
         this._animationBox.setAttribute("animate", "true");
@@ -9134,16 +9136,17 @@ var ConfirmationHint = {
   _reset() {
     if (this._timerID) {
       clearTimeout(this._timerID);
       this._timerID = null;
     }
     if (this.__panel) {
       this._panel.removeAttribute("hidearrow");
       this._animationBox.removeAttribute("animate");
+      this._panel.removeAttribute("data-message-id");
     }
   },
 
   get _panel() {
     this._ensurePanel();
     return this.__panel;
   },
 
--- a/browser/locales/en-US/chrome/browser/browser.properties
+++ b/browser/locales/en-US/chrome/browser/browser.properties
@@ -1034,16 +1034,17 @@ storageAccess2.message = Will you allow 
 
 confirmationHint.sendToDevice.label = Sent!
 confirmationHint.copyURL.label = Copied to clipboard!
 confirmationHint.pageBookmarked.label = Saved to Library!
 confirmationHint.addSearchEngine.label = Search engine added!
 confirmationHint.pinTab.label = Pinned!
 confirmationHint.pinTab.description = Right-click the tab to unpin it.
 confirmationHint.passwordSaved.label = Password saved!
+confirmationHint.loginRemoved.label = Login removed!
 confirmationHint.breakageReport.label = Report sent. Thank you!
 
 # LOCALIZATION NOTE (livebookmarkMigration.title):
 # Used by the export of user's live bookmarks to an OPML file as a title for the file.
 # %S will be replaced with brandShortName
 livebookmarkMigration.title                      = %S Live Bookmarks
 
 # LOCALIZATION NOTE (gnomeSearchProviderSearch):
--- a/browser/themes/shared/customizableui/panelUI.inc.css
+++ b/browser/themes/shared/customizableui/panelUI.inc.css
@@ -294,16 +294,24 @@ panelview {
   max-width: 266px;
   min-height: 14px;
   max-height: 14px;
   animation-name: confirmation-hint-checkmark-animation;
   animation-fill-mode: forwards;
   animation-timing-function: steps(18);
 }
 
+#confirmation-hint[data-message-id="loginRemoved"] #confirmation-hint-checkmark-image {
+  background-image: url("chrome://global/skin/icons/delete.svg");
+  background-size: contain;
+  -moz-context-properties: fill;
+  fill: #FFF;
+  animation: none;
+}
+
 #confirmation-hint-checkmark-animation-container[animate] > #confirmation-hint-checkmark-image:-moz-locale-dir(rtl) {
   animation-name: confirmation-hint-checkmark-animation-rtl;
   transform: translateX(252px);
 }
 
 @media (prefers-reduced-motion: no-preference) {
   #confirmation-hint-checkmark-animation-container[animate] > #confirmation-hint-checkmark-image {
     animation-duration: 300ms;
--- a/toolkit/components/passwordmgr/LoginManagerPrompter.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerPrompter.jsm
@@ -38,20 +38,20 @@ const BRAND_BUNDLE = "chrome://branding/
  * a saved login.
  */
 const VISIBILITY_TOGGLE_MAX_PW_AGE_MS = 2 * 60 * 1000; // 2 minutes
 
 /**
  * Constants for password prompt telemetry.
  */
 const PROMPT_DISPLAYED = 0;
-
 const PROMPT_ADD_OR_UPDATE = 1;
-const PROMPT_NOTNOW = 2;
+const PROMPT_NOTNOW_OR_DONTUPDATE = 2;
 const PROMPT_NEVER = 3;
+const PROMPT_DELETE = 3;
 
 /**
  * The minimum age of a doorhanger in ms before it will get removed after a locationchange
  */
 const NOTIFICATION_TIMEOUT_MS = 10 * 1000; // 10 seconds
 
 /**
  * The minimum age of an attention-requiring dismissed doorhanger in ms
@@ -401,17 +401,17 @@ class LoginManagerPrompter {
 
     let secondaryActions = [
       {
         label: this._getLocalizedString(initialMsgNames.secondaryButtonLabel),
         accessKey: this._getLocalizedString(
           initialMsgNames.secondaryButtonAccessKey
         ),
         callback: () => {
-          histogram.add(PROMPT_NOTNOW);
+          histogram.add(PROMPT_NOTNOW_OR_DONTUPDATE);
           Services.obs.notifyObservers(
             null,
             "weave:telemetry:histogram",
             histogramName
           );
           browser.focus();
         },
       },
@@ -429,16 +429,48 @@ class LoginManagerPrompter {
             histogramName
           );
           Services.logins.setLoginSavingEnabled(login.origin, false);
           browser.focus();
         },
       });
     }
 
+    // Include a "Delete this login" button when updating an existing password
+    if (type == "password-change") {
+      secondaryActions.push({
+        label: this._getLocalizedString("updateLoginButtonDelete.label"),
+        accessKey: this._getLocalizedString(
+          "updateLoginButtonDelete.accesskey"
+        ),
+        callback: async () => {
+          histogram.add(PROMPT_DELETE);
+          Services.obs.notifyObservers(
+            null,
+            "weave:telemetry:histogram",
+            histogramName
+          );
+          const matchingLogins = await Services.logins.searchLoginsAsync({
+            guid: login.guid,
+            origin: login.origin,
+          });
+          Services.logins.removeLogin(matchingLogins[0]);
+          browser.focus();
+          // The "password-notification-icon" and "notification-icon-box" are hidden
+          // at this point, so approximate the location with the next closest,
+          // visible icon as the anchor.
+          const anchor = browser.ownerDocument.getElementById("identity-icon");
+          log.debug("Showing the ConfirmationHint");
+          anchor.ownerGlobal.ConfirmationHint.show(anchor, "loginRemoved", {
+            hideArrow: true,
+          });
+        },
+      });
+    }
+
     let usernamePlaceholder = this._getLocalizedString("noUsernamePlaceholder");
     let togglePasswordLabel = this._getLocalizedString("togglePasswordLabel");
     let togglePasswordAccessKey = this._getLocalizedString(
       "togglePasswordAccessKey2"
     );
 
     // .wrappedJSObject needed here -- see bug 422974 comment 5.
     let { PopupNotifications } = browser.ownerGlobal.wrappedJSObject;
--- a/toolkit/components/passwordmgr/test/browser/browser_doorhanger_generated_password.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_doorhanger_generated_password.js
@@ -92,45 +92,16 @@ async function verifyGeneratedPasswordWa
         passwordInput.value.length,
         LTU.generation.LENGTH,
         "Password field was filled with generated password"
       );
     }
   );
 }
 
-async function verifyConfirmationHint(browser, forceClose) {
-  let hintElem = browser.ownerGlobal.ConfirmationHint._panel;
-  await BrowserTestUtils.waitForPopupEvent(hintElem, "shown");
-  try {
-    is(hintElem.state, "open", "hint popup is open");
-    ok(
-      BrowserTestUtils.is_visible(hintElem.anchorNode),
-      "hint anchorNode is visible"
-    );
-    is(
-      hintElem.anchorNode.id,
-      "password-notification-icon",
-      "Hint should be anchored on the password notification icon"
-    );
-    info("verifyConfirmationHint, hint is shown and has its anchorNode");
-    if (forceClose) {
-      await closePopup(hintElem);
-    } else {
-      info("verifyConfirmationHint, assertion ok, wait for poopuphidden");
-      await BrowserTestUtils.waitForPopupEvent(hintElem, "hidden");
-      info("verifyConfirmationHint, hintElem popup is hidden");
-    }
-  } catch (ex) {
-    ok(false, "Confirmation hint not shown: " + ex.message);
-  } finally {
-    info("verifyConfirmationHint promise finalized");
-  }
-}
-
 async function openFormInNewTab(url, formValues, taskFn) {
   let formFilled = listenForTestNotification("FormProcessed");
 
   await BrowserTestUtils.withNewTab(
     {
       gBrowser,
       url,
     },
--- a/toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_doorhanger_remembering.js
@@ -488,16 +488,48 @@ add_task(async function test_changeUPLog
   let login = logins[0].QueryInterface(Ci.nsILoginMetaInfo);
   is(login.username, "notifyu1", "Check the username unchanged");
   is(login.password, "notifyp1", "Check the password unchanged");
   is(login.timesUsed, 1, "Check times used");
 
   Services.logins.removeLogin(login1);
 });
 
+add_task(async function test_changeUPLoginOnUPForm_remove() {
+  info("Check for change-password popup, u+p login on u+p form. (remove)");
+  Services.logins.addLogin(login1);
+
+  await testSubmittingLoginForm("subtst_notifications_8.html", async function(
+    fieldValues
+  ) {
+    is(fieldValues.username, "notifyu1", "Checking submitted username");
+    is(fieldValues.password, "pass2", "Checking submitted password");
+    let notif = await getCaptureDoorhangerThatMayOpen("password-change");
+    ok(notif, "got notification popup");
+    ok(!notif.dismissed, "doorhanger is not dismissed");
+    is(notif.message, "Would you like to update this login?", "Check message");
+
+    await checkDoorhangerUsernamePassword("notifyu1", "pass2");
+    clickDoorhangerButton(notif, REMOVE_LOGIN_MENUITEM);
+  });
+
+  // Let the hint hide itself
+  const forceClosePopup = false;
+  // Make sure confirmation hint was shown
+  info("waiting for verifyConfirmationHint");
+  await verifyConfirmationHint(
+    gBrowser.selectedBrowser,
+    forceClosePopup,
+    "identity-icon"
+  );
+
+  let logins = Services.logins.getAllLogins();
+  is(logins.length, 0, "Should have 0 logins");
+});
+
 add_task(async function test_changeUPLoginOnUPForm_change() {
   info("Check for change-password popup, u+p login on u+p form.");
   Services.logins.addLogin(login1);
 
   await testSubmittingLoginForm("subtst_notifications_8.html", async function(
     fieldValues
   ) {
     is(fieldValues.username, "notifyu1", "Checking submitted username");
--- a/toolkit/components/passwordmgr/test/browser/head.js
+++ b/toolkit/components/passwordmgr/test/browser/head.js
@@ -260,16 +260,17 @@ function clearHttpAuths() {
 
 // Begin popup notification (doorhanger) functions //
 
 const REMEMBER_BUTTON = "button";
 const NEVER_MENUITEM = 0;
 
 const CHANGE_BUTTON = "button";
 const DONT_CHANGE_BUTTON = "secondaryButton";
+const REMOVE_LOGIN_MENUITEM = 0;
 
 /**
  * Checks if we have a password capture popup notification
  * of the right type and with the right label.
  *
  * @param {String} aKind The desired `passwordNotificationType` ("any" for any type)
  * @param {Object} [popupNotifications = PopupNotifications]
  * @param {Object} [browser = null] Optional browser whose notifications should be searched.
@@ -778,8 +779,41 @@ async function changeContentInputValue(b
     input.blur();
     await changedPromise;
 
     is(str, input.value, `Expected value '${str}' is set on input`);
   });
   info("Input value changed");
   await TestUtils.waitForTick();
 }
+
+async function verifyConfirmationHint(
+  browser,
+  forceClose,
+  anchorID = "password-notification-icon"
+) {
+  let hintElem = browser.ownerGlobal.ConfirmationHint._panel;
+  await BrowserTestUtils.waitForPopupEvent(hintElem, "shown");
+  try {
+    is(hintElem.state, "open", "hint popup is open");
+    ok(
+      BrowserTestUtils.is_visible(hintElem.anchorNode),
+      "hint anchorNode is visible"
+    );
+    is(
+      hintElem.anchorNode.id,
+      anchorID,
+      "Hint should be anchored on the expected notification icon"
+    );
+    info("verifyConfirmationHint, hint is shown and has its anchorNode");
+    if (forceClose) {
+      await closePopup(hintElem);
+    } else {
+      info("verifyConfirmationHint, assertion ok, wait for poopuphidden");
+      await BrowserTestUtils.waitForPopupEvent(hintElem, "hidden");
+      info("verifyConfirmationHint, hintElem popup is hidden");
+    }
+  } catch (ex) {
+    ok(false, "Confirmation hint not shown: " + ex.message);
+  } finally {
+    info("verifyConfirmationHint promise finalized");
+  }
+}
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -12332,19 +12332,19 @@
     "record_into_store": ["main", "pre-account"]
   },
   "PWMGR_PROMPT_UPDATE_ACTION" : {
     "record_in_processes": ["main"],
     "products": ["firefox", "fennec", "geckoview"],
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 5,
-    "description": "Action taken by user through prompt for modifying a login. (0=Prompt displayed [always recorded], 1=Update login)",
+    "description": "Action taken by user through prompt for modifying a login. (0=Prompt displayed [always recorded], 1=Update login, 2=Don't update, 3=Remove saved login)",
     "alert_emails": ["loines@mozilla.com"],
-    "bug_numbers": [1454733, 1545172],
+    "bug_numbers": [1454733, 1545172, 1226236],
     "releaseChannelCollection": "opt-out",
     "record_into_store": ["main", "pre-account"]
   },
   "PWMGR_SAVING_ENABLED": {
     "record_in_processes": ["main"],
     "products": ["firefox", "fennec", "geckoview", "thunderbird"],
     "expires_in_version": "never",
     "kind": "boolean",
--- a/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
+++ b/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
@@ -16,16 +16,18 @@ saveLoginButtonNever.label = Never Save
 saveLoginButtonNever.accesskey = e
 updateLoginMsg = Would you like to update this login?
 updateLoginMsgNoUser = Would you like to update this password?
 updateLoginMsgAddUsername = Would you like to add a username to the saved password?
 updateLoginButtonText = Update
 updateLoginButtonAccessKey = U
 updateLoginButtonDeny.label = Don’t Update
 updateLoginButtonDeny.accesskey = D
+updateLoginButtonDelete.label = Remove Saved Login
+updateLoginButtonDelete.accesskey = R
 # LOCALIZATION NOTE (rememberPasswordMsg):
 # 1st string is the username for the login, 2nd is the login's hostname.
 # Note that long usernames may be truncated.
 rememberPasswordMsg = Would you like to remember the password for “%1$S” on %2$S?
 # LOCALIZATION NOTE (rememberPasswordMsgNoUsername):
 # String is the login's hostname.
 rememberPasswordMsgNoUsername = Would you like to remember the password on %S?
 # LOCALIZATION NOTE (noUsernamePlaceholder):