Bug 1569723 - Show a 'Discard' dialog when canceling or leaving the edit/create mode in about:logins. r=fluent-reviewers,jaws,flod
authorTim Nguyen <ntim.bugs@gmail.com>
Thu, 01 Aug 2019 03:09:00 +0000
changeset 485714 aa73bf741807e1284e446ad622cec2ade201c365
parent 485713 f869dc3ce4a9853a84077daf9a3db6d0af771c6d
child 485715 39cae66f74cb21356c81cbb92d94ba7af5b3d3ea
push id36373
push userrmaries@mozilla.com
push dateFri, 02 Aug 2019 03:50:33 +0000
treeherdermozilla-central@22bda3da7ce3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfluent-reviewers, jaws, flod
bugs1569723
milestone70.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 1569723 - Show a 'Discard' dialog when canceling or leaving the edit/create mode in about:logins. r=fluent-reviewers,jaws,flod Differential Revision: https://phabricator.services.mozilla.com/D39984
browser/components/aboutlogins/content/components/login-item.js
browser/components/aboutlogins/content/components/login-list.js
browser/components/aboutlogins/tests/browser/browser_updateLogin.js
browser/components/aboutlogins/tests/chrome/aboutlogins_common.js
browser/components/aboutlogins/tests/chrome/test_login_item.html
browser/locales/en-US/browser/aboutLogins.ftl
--- a/browser/components/aboutlogins/content/components/login-item.js
+++ b/browser/components/aboutlogins/content/components/login-item.js
@@ -121,17 +121,33 @@ export default class LoginItem extends H
         this.setLogin({});
         break;
       }
       case "AboutLoginsInitialLoginSelected": {
         this.setLogin(event.detail, { skipFocusChange: true });
         break;
       }
       case "AboutLoginsLoginSelected": {
-        this.setLogin(event.detail);
+        let login = event.detail;
+        if (this.hasPendingChanges()) {
+          event.preventDefault();
+          this.showConfirmationDialog("discard-changes", () => {
+            // Clear any pending changes
+            this.setLogin(login);
+
+            window.dispatchEvent(
+              new CustomEvent("AboutLoginsLoginSelected", {
+                detail: login,
+                cancelable: true,
+              })
+            );
+          });
+        } else {
+          this.setLogin(login);
+        }
         break;
       }
       case "blur": {
         // Add https:// prefix if one was not provided.
         let originValue = this._originInput.value.trim();
         if (!originValue) {
           return;
         }
@@ -148,17 +164,23 @@ export default class LoginItem extends H
           let method = this._revealCheckbox.checked ? "show" : "hide";
           recordTelemetryEvent({ object: "password", method });
           return;
         }
 
         if (classList.contains("cancel-button")) {
           let wasExistingLogin = !!this._login.guid;
           if (wasExistingLogin) {
-            this.setLogin(this._login);
+            if (this.hasPendingChanges()) {
+              this.showConfirmationDialog("discard-changes", () => {
+                this.setLogin(this._login);
+              });
+            } else {
+              this.setLogin(this._login);
+            }
           } else {
             window.dispatchEvent(new CustomEvent("AboutLoginsClearSelection"));
           }
 
           recordTelemetryEvent({
             object: wasExistingLogin ? "existing_login" : "new_login",
             method: "cancel",
           });
@@ -269,16 +291,25 @@ export default class LoginItem extends H
     let options;
     switch (type) {
       case "delete": {
         options = {
           title: "confirm-delete-dialog-title",
           message: "confirm-delete-dialog-message",
           confirmButtonLabel: "confirm-delete-dialog-confirm-button",
         };
+        break;
+      }
+      case "discard-changes": {
+        options = {
+          title: "confirm-discard-changes-dialog-title",
+          message: "confirm-discard-changes-dialog-message",
+          confirmButtonLabel: "confirm-discard-changes-dialog-confirm-button",
+        };
+        break;
       }
     }
     let dialogPromise = dialog.show(options);
     dialogPromise.then(onConfirm, () => {});
     return dialogPromise;
   }
 
   /**
@@ -292,16 +323,27 @@ export default class LoginItem extends H
           bubbles: true,
           detail: this._login,
         })
       );
       recordTelemetryEvent({ object: "existing_login", method: "delete" });
     });
   }
 
+  hasPendingChanges() {
+    let { origin = "", username = "", password = "" } = this._login || {};
+
+    let valuesChanged = !window.AboutLoginsUtils.doLoginsMatch(
+      { origin, username, password },
+      this._loginFromForm()
+    );
+
+    return this.dataset.editing && valuesChanged;
+  }
+
   /**
    * @param {login} login The login that should be displayed. The login object is
    *                      a plain JS object representation of nsILoginInfo/nsILoginMetaInfo.
    * @param {boolean} skipFocusChange Optional, if present and set to true, the Edit button of the
    *                                  login will not get focus automatically. This is used to prevent
    *                                  stealing focus from the search filter upon page load.
    */
   setLogin(login, { skipFocusChange } = {}) {
--- a/browser/components/aboutlogins/content/components/login-list.js
+++ b/browser/components/aboutlogins/content/components/login-list.js
@@ -113,16 +113,17 @@ export default class LoginList extends H
         if (!listItem || !listItem.dataset.guid) {
           return;
         }
 
         this.dispatchEvent(
           new CustomEvent("AboutLoginsLoginSelected", {
             bubbles: true,
             composed: true,
+            cancelable: true, // allow calling preventDefault() on event
             detail: listItem._login,
           })
         );
 
         recordTelemetryEvent({ object: "existing_login", method: "select" });
         break;
       }
       case "change": {
@@ -132,32 +133,33 @@ export default class LoginList extends H
       }
       case "AboutLoginsClearSelection": {
         if (!this._loginGuidsSortedOrder.length) {
           return;
         }
         window.dispatchEvent(
           new CustomEvent("AboutLoginsLoginSelected", {
             detail: this._logins[0],
+            cancelable: true,
           })
         );
         break;
       }
       case "AboutLoginsCreateLogin": {
         this._selectedGuid = null;
         this._setListItemAsSelected(this._blankLoginListItem);
         break;
       }
       case "AboutLoginsFilterLogins": {
         this._filter = event.detail.toLocaleLowerCase();
         this.render();
         break;
       }
       case "AboutLoginsLoginSelected": {
-        if (this._selectedGuid == event.detail.guid) {
+        if (event.defaultPrevented || this._selectedGuid == event.detail.guid) {
           return;
         }
 
         let listItem = this._list.querySelector(
           `.login-list-item[data-guid="${event.detail.guid}"]`
         );
         if (listItem) {
           this._setListItemAsSelected(listItem);
--- a/browser/components/aboutlogins/tests/browser/browser_updateLogin.js
+++ b/browser/components/aboutlogins/tests/browser/browser_updateLogin.js
@@ -59,24 +59,48 @@ add_task(async function test_login_item(
 
       let editButton = loginItem.shadowRoot.querySelector(".edit-button");
       editButton.click();
       await Promise.resolve();
 
       usernameInput.value += "-undome";
       passwordInput.value += "-undome";
 
+      let dialog = content.document.querySelector("confirmation-dialog");
+      ok(dialog.hidden, "Confirm dialog should initially be hidden");
+
       let cancelButton = loginItem.shadowRoot.querySelector(".cancel-button");
       cancelButton.click();
+
+      ok(!dialog.hidden, "Confirm dialog should be visible");
+
+      let confirmDiscardButton = dialog.shadowRoot.querySelector(
+        ".confirm-button"
+      );
+      await content.document.l10n.translateElements([
+        dialog.shadowRoot.querySelector(".title"),
+        dialog.shadowRoot.querySelector(".message"),
+        confirmDiscardButton,
+      ]);
+
+      confirmDiscardButton.click();
+
+      ok(dialog.hidden, "Confirm dialog should be hidden after confirming");
+
       usernameInput = loginItem.shadowRoot.querySelector(
         "input[name='username']"
       );
       passwordInput = loginItem.shadowRoot.querySelector(
         "input[name='password']"
       );
+
+      await ContentTaskUtils.waitForCondition(
+        () => usernameInput.value == login.username
+      );
+
       is(
         usernameInput.value,
         login.username,
         "Username change should be reverted"
       );
       is(
         passwordInput.value,
         login.password,
--- a/browser/components/aboutlogins/tests/chrome/aboutlogins_common.js
+++ b/browser/components/aboutlogins/tests/chrome/aboutlogins_common.js
@@ -54,10 +54,17 @@ Object.defineProperty(document, "l10n", 
 
 Object.defineProperty(window, "AboutLoginsUtils", {
   configurable: true,
   writable: true,
   value: {
     promptForMasterPassword(resolve) {
       resolve(true);
     },
+    doLoginsMatch(login1, login2) {
+      return (
+        login1.origin == login2.origin &&
+        login1.username == login2.username &&
+        login1.password == login2.password
+      );
+    },
   },
 });
--- a/browser/components/aboutlogins/tests/chrome/test_login_item.html
+++ b/browser/components/aboutlogins/tests/chrome/test_login_item.html
@@ -4,33 +4,34 @@
 Test the login-item component
 -->
 <head>
   <meta charset="utf-8">
   <title>Test the login-item component</title>
   <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
   <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"></script>
   <script type="module" src="chrome://browser/content/aboutlogins/components/login-item.js"></script>
+  <script type="module" src="chrome://browser/content/aboutlogins/components/confirmation-dialog.js"></script>
   <script src="aboutlogins_common.js"></script>
 
   <link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
   <p id="display">
   </p>
 <div id="content" style="display: none">
   <iframe id="templateFrame" src="chrome://browser/content/aboutlogins/aboutLogins.html"
           sandbox="allow-same-origin"></iframe>
 </div>
 <pre id="test">
 </pre>
 <script>
 /** Test the login-item component **/
 
-let gLoginItem;
+let gLoginItem, gConfirmationDialog;
 const TEST_LOGIN_1 = {
   guid: "123456789",
   origin: "https://example.com",
   username: "user1",
   password: "pass1",
   timeCreated: "1000",
   timePasswordChanged: "2000",
   timeLastUsed: "4000",
@@ -56,16 +57,20 @@ TEST_BREACHES_MAP.set(TEST_LOGIN_1.guid,
 
 add_task(async function setup() {
   let templateFrame = document.getElementById("templateFrame");
   let displayEl = document.getElementById("display");
   importDependencies(templateFrame, displayEl);
 
   gLoginItem = document.createElement("login-item");
   displayEl.appendChild(gLoginItem);
+
+  gConfirmationDialog = document.createElement("confirmation-dialog");
+  gConfirmationDialog.hidden = true;
+  displayEl.appendChild(gConfirmationDialog);
 });
 
 add_task(async function test_empty_item() {
   ok(gLoginItem, "loginItem exists");
   is(gLoginItem.shadowRoot.querySelector("input[name='origin']").value, "", "origin should be blank");
   is(gLoginItem.shadowRoot.querySelector("input[name='username']").value, "", "username should be blank");
   is(gLoginItem.shadowRoot.querySelector("input[name='password']").value, "", "password should be blank");
   is(document.l10n.getAttributes(gLoginItem.shadowRoot.querySelector(".time-created")).args.timeCreated, "", "time-created should be blank when undefined");
@@ -147,16 +152,23 @@ add_task(async function test_edit_login_
   gLoginItem.setLogin(TEST_LOGIN_1);
   gLoginItem.shadowRoot.querySelector(".edit-button").click();
 
   ok(gLoginItem.dataset.editing, "loginItem should be in 'edit' mode");
   is(!!gLoginItem.dataset.isNewLogin, false,
      "loginItem should not be in 'isNewLogin' mode");
 
   gLoginItem.shadowRoot.querySelector(".cancel-button").click();
+  gConfirmationDialog.shadowRoot.querySelector(".confirm-button").click();
+
+  await SimpleTest.promiseWaitForCondition(
+    () => gConfirmationDialog.hidden,
+    "waiting for confirmation dialog to hide"
+  );
+
   ok(!gLoginItem.dataset.editing, "loginItem should not be in 'edit' mode");
   ok(!gLoginItem.dataset.isNewLogin, "loginItem should not be in 'isNewLogin' mode");
 });
 
 add_task(async function test_reveal_password_change_selected_login() {
   gLoginItem.setLogin(TEST_LOGIN_1);
   let revealCheckbox = gLoginItem.shadowRoot.querySelector(".reveal-password-checkbox");
   let passwordInput = gLoginItem.shadowRoot.querySelector("input[name='password']");
--- a/browser/locales/en-US/browser/aboutLogins.ftl
+++ b/browser/locales/en-US/browser/aboutLogins.ftl
@@ -98,12 +98,16 @@ enable-password-sync-preferences-button 
        *[other] Visit { -sync-brand-short-name } Preferences
     }
   .accesskey = V
 
 confirm-delete-dialog-title = Delete this login?
 confirm-delete-dialog-message = This action cannot be undone.
 confirm-delete-dialog-confirm-button = Delete
 
+confirm-discard-changes-dialog-title = Discard unsaved changes?
+confirm-discard-changes-dialog-message = All unsaved changes will be lost.
+confirm-discard-changes-dialog-confirm-button = Discard
+
 ## Breach Alert notification
 
 breach-alert-text = Passwords were leaked or stolen from this website since you last updated your login details. Change your password to protect your account.
 breach-alert-link = Learn more about this breach.