Bug 1550122: Show errors in aboutLogins UI. r=MattN,fluent-reviewers,ntim,flod

Differential Revision: https://phabricator.services.mozilla.com/D42705

--HG--
extra : moz-landing-system : lando

browser/components/BrowserGlue.jsm

@@ -138,6 +138,7 @@ let LEGACY_ACTORS = {
         "AboutLogins:LoginRemoved",
         "AboutLogins:MasterPasswordResponse",
         "AboutLogins:SendFavicons",
+        "AboutLogins:ShowLoginItemError",
         "AboutLogins:SyncState",
         "AboutLogins:UpdateBreaches",
       ],

browser/components/aboutlogins/AboutLoginsChild.jsm

@@ -187,6 +187,9 @@ class AboutLoginsChild extends ActorChild {
       case "AboutLogins:SendFavicons":
         this.sendToContent("SendFavicons", message.data);
         break;
+      case "AboutLogins:ShowLoginItemError":
+        this.sendToContent("ShowLoginItemError", message.data);
+        break;
       case "AboutLogins:SyncState":
         this.sendToContent("SyncState", message.data);
         break;

browser/components/aboutlogins/AboutLoginsParent.jsm

@@ -101,7 +101,12 @@ var AboutLoginsParent = {
           usernameField: "",
           passwordField: "",
         });
-        Services.logins.addLogin(LoginHelper.vanillaObjectToLogin(newLogin));
+        newLogin = LoginHelper.vanillaObjectToLogin(newLogin);
+        try {
+          Services.logins.addLogin(newLogin);
+        } catch (error) {
+          this.handleLoginStorageErrors(newLogin, error, message);
+        }
         break;
       }
       case "AboutLogins:DeleteLogin": {
@@ -459,13 +464,25 @@ var AboutLoginsParent = {
         if (loginUpdates.hasOwnProperty("password")) {
           modifiedLogin.password = loginUpdates.password;
         }
-
-        Services.logins.modifyLogin(logins[0], modifiedLogin);
+        try {
+          Services.logins.modifyLogin(logins[0], modifiedLogin);
+        } catch (error) {
+          this.handleLoginStorageErrors(modifiedLogin, error, message);
+        }
         break;
       }
     }
   },
 
+  handleLoginStorageErrors(login, error, message) {
+    const messageManager = message.target.messageManager;
+    const errorMessage = error.message;
+    messageManager.sendAsyncMessage("AboutLogins:ShowLoginItemError", {
+      login: augmentVanillaLoginObject(login),
+      errorMessage,
+    });
+  },
+
   async observe(subject, topic, type) {
     if (!ChromeUtils.nondeterministicGetWeakSetKeys(this._subscribers).length) {
       Services.obs.removeObserver(this, "passwordmgr-crypto-login");

browser/components/aboutlogins/content/aboutLogins.html

@@ -141,6 +141,9 @@
       <link rel="stylesheet" href="chrome://global/skin/in-content/common.css">
       <link rel="stylesheet" href="chrome://browser/content/aboutlogins/common.css">
       <link rel="stylesheet" href="chrome://browser/content/aboutlogins/components/login-item.css">
+      <div class="error-message">
+        <span class="error-message-text"></span>
+      </div>
       <div class="breach-alert">
         <span class="breach-alert-text" data-l10n-id="breach-alert-text"></span>
         <a class="breach-alert-link" data-l10n-id="breach-alert-link" href="#" rel="noopener noreferer" target="_blank"></a>

browser/components/aboutlogins/content/aboutLogins.js

@@ -66,6 +66,10 @@ window.addEventListener("AboutLoginsChromeToContent", event => {
       gElements.loginList.addFavicons(event.detail.value);
       break;
     }
+    case "ShowLoginItemError": {
+      gElements.loginItem.showLoginItemError(event.detail.value);
+      break;
+    }
     case "SyncState": {
       gElements.fxAccountsButton.updateState(event.detail.value);
       gElements.loginFooter.hidden = event.detail.value.hideMobileFooter;

browser/components/aboutlogins/content/components/login-item.css

@@ -294,6 +294,26 @@ a.breach-alert-link {
   background-color: transparent;
 }
 
+.error-message {
+  color: #fff;
+  background-color: var(--red-60);
+  border: 1px solid transparent;
+  padding-block: 6px;
+  display: inline-block;
+  padding-inline-start: 32px;
+  padding-inline-end: 16px;
+  background-image: url("chrome://global/skin/icons/warning.svg");
+  background-repeat: no-repeat;
+  background-position: left 10px center;
+  -moz-context-properties: fill;
+  fill: currentColor;
+  margin-bottom: 38px;
+}
+
+.error-message:dir(rtl) {
+  background-position-x: right 10px;
+}
+
 @supports -moz-bool-pref("browser.in-content.dark-mode") {
   @media (prefers-color-scheme: dark) {
     :host {

browser/components/aboutlogins/content/components/login-item.js

@@ -41,6 +41,7 @@ export default class LoginItem extends HTMLElement {
     );
     this._deleteButton = this.shadowRoot.querySelector(".delete-button");
     this._editButton = this.shadowRoot.querySelector(".edit-button");
+    this._errorMessage = this.shadowRoot.querySelector(".error-message");
     this._form = this.shadowRoot.querySelector("form");
     this._originInput = this.shadowRoot.querySelector("input[name='origin']");
     this._usernameInput = this.shadowRoot.querySelector(
@@ -88,7 +89,9 @@ export default class LoginItem extends HTMLElement {
   }
 
   async render() {
-    this._breachAlert.hidden = true;
+    [this._errorMessage, this._breachAlert].forEach(el => {
+      el.hidden = true;
+    });
     if (this._breachesMap && this._breachesMap.has(this._login.guid)) {
       const breachDetails = this._breachesMap.get(this._login.guid);
       const breachAlertLink = this._breachAlert.querySelector(
@@ -160,6 +163,30 @@ export default class LoginItem extends HTMLElement {
     );
   }
 
+  showLoginItemError(error) {
+    const errorMessageText = this._errorMessage.querySelector(
+      ".error-message-text"
+    );
+    if (!error.errorMessage) {
+      return;
+    }
+    if (error.errorMessage.includes("This login already exists")) {
+      document.l10n.setAttributes(
+        errorMessageText,
+        "about-logins-error-message-duplicate-login",
+        {
+          loginTitle: error.login.title,
+        }
+      );
+    } else {
+      document.l10n.setAttributes(
+        errorMessageText,
+        "about-logins-error-message-default"
+      );
+    }
+    this._errorMessage.hidden = false;
+  }
+
   async handleEvent(event) {
     switch (event.type) {
       case "AboutLoginsInitialLoginSelected": {

browser/components/aboutlogins/tests/browser/browser.ini

@@ -17,6 +17,7 @@ skip-if = asan || debug || verify # bug 1574023
 [browser_deleteLogin.js]
 [browser_dismissFooter.js]
 [browser_fxAccounts.js]
+[browser_loginItemErrors.js]
 [browser_loginListChanges.js]
 [browser_masterPassword.js]
 skip-if = (os == 'linux') # bug 1569789

browser/components/aboutlogins/tests/browser/browser_loginItemErrors.js

@@ -0,0 +1,126 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+add_task(async function setup() {
+  await BrowserTestUtils.openNewForegroundTab({
+    gBrowser,
+    url: "about:logins",
+  });
+  registerCleanupFunction(() => {
+    BrowserTestUtils.removeTab(gBrowser.selectedTab);
+    Services.logins.removeAllLogins();
+  });
+});
+
+add_task(async function test_showLoginItemErrors() {
+  const browser = gBrowser.selectedBrowser;
+  let LOGIN_TO_UPDATE = new nsLoginInfo(
+    "https://example.com/",
+    "https://example.com/",
+    null,
+    "user2",
+    "pass2",
+    "username",
+    "password"
+  );
+  LOGIN_TO_UPDATE = Services.logins.addLogin(LOGIN_TO_UPDATE);
+
+  await ContentTask.spawn(
+    browser,
+    LoginHelper.loginToVanillaObject(LOGIN_TO_UPDATE),
+    async loginToUpdate => {
+      const loginItem = Cu.waiveXrays(
+        content.document.querySelector("login-item")
+      );
+      const loginItemErrorMessage = Cu.waiveXrays(
+        loginItem.shadowRoot.querySelector(".error-message")
+      );
+      const loginList = Cu.waiveXrays(
+        content.document.querySelector("login-list")
+      );
+
+      const createButton = loginList._createLoginButton;
+      createButton.click();
+
+      const loginUpdates = {
+        origin: "https://example.com/",
+        password: "my1GoodPassword",
+        username: "user1",
+      };
+
+      const event = Cu.cloneInto(
+        {
+          bubbles: true,
+          detail: loginUpdates,
+        },
+        content
+      );
+
+      content.dispatchEvent(
+        // adds first lgoin
+        new content.CustomEvent("AboutLoginsCreateLogin", event)
+      );
+
+      ok(
+        loginItemErrorMessage.hidden,
+        "An error message should not be displayed after adding a new login."
+      );
+
+      content.dispatchEvent(
+        // adds a duplicate of the first login
+        new content.CustomEvent("AboutLoginsCreateLogin", event)
+      );
+
+      const loginItemErrorMessageVisible = await ContentTaskUtils.waitForCondition(
+        () => {
+          return !loginItemErrorMessage.hidden;
+        },
+        "Waiting for error message to be shown after attempting to create a duplicate login."
+      );
+      ok(
+        loginItemErrorMessageVisible,
+        "An error message should be shown after user attempts to add a login that already exists."
+      );
+
+      const loginItemErrorMessageText = loginItemErrorMessage.querySelector(
+        "span"
+      );
+      ok(
+        loginItemErrorMessageText.dataset.l10nId ===
+          "about-logins-error-message-duplicate-login",
+        "The correct error message is displayed."
+      );
+
+      let loginListItem = Cu.waiveXrays(
+        loginList.shadowRoot.querySelector(
+          `.login-list-item[data-guid='${loginToUpdate.guid}']`
+        )
+      );
+      loginListItem.click();
+
+      ok(
+        loginItemErrorMessage.hidden,
+        "The error message should no longer be visible."
+      );
+
+      const editButton = loginItem.shadowRoot.querySelector(".edit-button");
+      editButton.click();
+
+      content.dispatchEvent(
+        // attempt to update LOGIN_TO_UPDATE to a username/origin combination that already exists.
+        new content.CustomEvent("AboutLoginsUpdateLogin", event)
+      );
+
+      const loginAlreadyExistsErrorShownAfterUpdate = ContentTaskUtils.waitForCondition(
+        () => {
+          return !loginItemErrorMessage.hidden;
+        },
+        "Waiting for error message to show after updating login to existing login."
+      );
+      ok(
+        loginAlreadyExistsErrorShownAfterUpdate,
+        "An error message should be shown after updating a login to a username/origin combination that already exists."
+      );
+    }
+  );
+});

browser/locales/en-US/browser/aboutLogins.ftl

@@ -141,3 +141,14 @@ breach-alert-text = Passwords were leaked or stolen from this website since you
 breach-alert-link = Learn more about this breach.
 breach-alert-dismiss =
     .title = Close this alert
+
+## Error Messages
+
+# This is an error message that appears when a user attempts to save
+# a new login that is identical to an existing saved login.
+# Variables:
+#   $loginTitle (String) - The title of the website associated with the login.
+about-logins-error-message-duplicate-login = An entry for { $loginTitle } with that username already exists.
+
+# This is a generic error message.
+about-logins-error-message-default = An error occurred while trying to save this password.