Bug 1557533 - Show a 'New Login' list item when creating a new login. r=fluent-reviewers,Pike
authorJared Wein <jwein@mozilla.com>
Wed, 12 Jun 2019 14:44:28 +0000
changeset 478437 080c52c4f2a09593131f70aeda4f72dd50aa59a3
parent 478436 6b07113a35553ccecf66546f3b2bd8eebc080800
child 478438 29f19462d77837f7ef982b7687e2c794e3d95f87
push id36143
push useraciure@mozilla.com
push dateWed, 12 Jun 2019 21:41:19 +0000
treeherdermozilla-central@32de23ddd8e2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfluent-reviewers, Pike
bugs1557533
milestone69.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 1557533 - Show a 'New Login' list item when creating a new login. r=fluent-reviewers,Pike Differential Revision: https://phabricator.services.mozilla.com/D34115
browser/components/aboutlogins/content/aboutLogins.ftl
browser/components/aboutlogins/content/aboutLogins.html
browser/components/aboutlogins/content/aboutLogins.js
browser/components/aboutlogins/content/components/login-list-item.js
browser/components/aboutlogins/content/components/login-list.js
browser/components/aboutlogins/tests/browser/browser_aaa_eventTelemetry_run_first.js
browser/components/aboutlogins/tests/browser/browser_deleteLogin.js
browser/components/aboutlogins/tests/browser/browser_openFiltered.js
browser/components/aboutlogins/tests/browser/browser_updateLogin.js
browser/components/aboutlogins/tests/chrome/test_login_list.html
--- a/browser/components/aboutlogins/content/aboutLogins.ftl
+++ b/browser/components/aboutlogins/content/aboutLogins.ftl
@@ -15,35 +15,38 @@ about-logins-page-title = Logins & Passw
 create-login-button = New Login
 
 login-filter =
   .placeholder = Search Logins
 
 login-list =
   .count =
     { $count ->
-        [one] { $count } entry
-       *[other] { $count } entries
+        [one] { $count } login
+       *[other] { $count } logins
     }
   .last-changed-option = Last Changed
   .last-used-option = Last Used
   .name-option = Name
+  .new-login-subtitle = Enter your login credentials
+  .new-login-title = New Login
   .sort-label-text = Sort by:
+
 login-item =
   .cancel-button = Cancel
   .copied-password-button = ✓ Copied!
   .copied-username-button = ✓ Copied!
   .copy-password-button = Copy
   .copy-username-button = Copy
   .delete-button = Delete
   .edit-button = Edit
   .field-required-symbol = *
   .modal-input-reveal-checkbox-hide = Hide password
   .modal-input-reveal-checkbox-show = Show password
-  .new-login-title = New Entry
+  .new-login-title = Create New Login
   .open-site-button = Launch
   .origin-label = Website Address
   .password-label = Password
   .save-changes-button = Save Changes
   .time-created = Created: { DATETIME($timeCreated, day: "numeric", month: "long", year: "numeric") }
   .time-changed = Last modified: { DATETIME($timeChanged, day: "numeric", month: "long", year: "numeric") }
   .time-used = Last used: { DATETIME($timeUsed, day: "numeric", month: "long", year: "numeric") }
   .username-label = Username
--- a/browser/components/aboutlogins/content/aboutLogins.html
+++ b/browser/components/aboutlogins/content/aboutLogins.html
@@ -31,16 +31,18 @@
                                     menuitem-preferences"></menu-button>
     </header>
     <login-list data-l10n-id="login-list"
                 data-l10n-args='{"count": 0}'
                 data-l10n-attrs="count,
                                  last-changed-option,
                                  last-used-option,
                                  name-option,
+                                 new-login-subtitle,
+                                 new-login-title,
                                  sort-label-text"></login-list>
     <login-item data-l10n-id="login-item"
                 data-l10n-args='{"timeCreated": 0, "timeChanged": 0, "timeUsed": 0}'
                 data-l10n-attrs="cancel-button,
                                  copy-password-button,
                                  copy-username-button,
                                  copied-password-button,
                                  copied-username-button,
--- a/browser/components/aboutlogins/content/aboutLogins.js
+++ b/browser/components/aboutlogins/content/aboutLogins.js
@@ -13,18 +13,19 @@ document.addEventListener("DOMContentLoa
   gElements.newLoginButton = document.querySelector("#create-login-button");
 
   let {searchParams} = new URL(document.location);
   if (searchParams.get("filter")) {
     gElements.loginFilter.value = searchParams.get("filter");
   }
 
   gElements.newLoginButton.addEventListener("click", () => {
-    gElements.loginItem.setLogin({});
-    gElements.loginList.clearSelection();
+    window.dispatchEvent(new CustomEvent("AboutLoginsLoginSelected", {
+      detail: {},
+    }));
 
     recordTelemetryEvent({object: "new_login", method: "new"});
   });
 
   document.dispatchEvent(new CustomEvent("AboutLoginsInit", {bubbles: true}));
 }, {once: true});
 
 window.addEventListener("AboutLoginsChromeToContent", event => {
--- a/browser/components/aboutlogins/content/components/login-list-item.js
+++ b/browser/components/aboutlogins/content/components/login-list-item.js
@@ -20,19 +20,29 @@ export default class LoginListItem exten
     this.attachShadow({mode: "open"})
         .appendChild(loginListItemTemplate.content.cloneNode(true));
     this.render();
 
     this.addEventListener("click", this);
   }
 
   render() {
+    let origin = this.shadowRoot.querySelector(".origin");
+    let username = this.shadowRoot.querySelector(".username");
+
+    if (!this._login.guid) {
+      this.removeAttribute("guid");
+      origin.textContent = this.getAttribute("new-login-title");
+      username.textContent = this.getAttribute("new-login-subtitle");
+      return;
+    }
+
     this.setAttribute("guid", this._login.guid);
-    this.shadowRoot.querySelector(".origin").textContent = this._login.origin;
-    this.shadowRoot.querySelector(".username").textContent = this._login.username;
+    origin.textContent = this._login.origin;
+    username.textContent = this._login.username;
   }
 
   handleEvent(event) {
     switch (event.type) {
       case "click": {
         this.dispatchEvent(new CustomEvent("AboutLoginsLoginSelected", {
           bubbles: true,
           composed: true,
--- a/browser/components/aboutlogins/content/components/login-list.js
+++ b/browser/components/aboutlogins/content/components/login-list.js
@@ -13,16 +13,17 @@ const sortFnOptions = {
 };
 
 export default class LoginList extends ReflectedFluentElement {
   constructor() {
     super();
     this._logins = [];
     this._filter = "";
     this._selectedGuid = null;
+    this._blankLoginListItem = new LoginListItem({});
   }
 
   connectedCallback() {
     if (this.shadowRoot) {
       return;
     }
     let loginListTemplate = document.querySelector("#login-list-template");
     this.attachShadow({mode: "open"})
@@ -36,18 +37,33 @@ export default class LoginList extends R
                    .addEventListener("change", this);
     window.addEventListener("AboutLoginsLoginSelected", this);
     window.addEventListener("AboutLoginsFilterLogins", this);
   }
 
   render() {
     let list = this.shadowRoot.querySelector("ol");
     list.textContent = "";
+
+    if (!this._logins.length) {
+      document.l10n.setAttributes(this, "login-list", {count: 0});
+      return;
+    }
+
+    if (!this._selectedGuid) {
+      this._blankLoginListItem.classList.add("selected");
+      list.append(this._blankLoginListItem);
+    }
+
     for (let login of this._logins) {
-      list.append(new LoginListItem(login));
+      let listItem = new LoginListItem(login);
+      if (login.guid == this._selectedGuid) {
+        listItem.classList.add("selected");
+      }
+      list.append(listItem);
     }
 
     let visibleLoginCount = this._applyFilter();
     document.l10n.setAttributes(this, "login-list", {count: visibleLoginCount});
   }
 
   _applyFilter() {
     let matchingLoginGuids;
@@ -56,16 +72,20 @@ export default class LoginList extends R
         return login.origin.toLocaleLowerCase().includes(this._filter) ||
                login.username.toLocaleLowerCase().includes(this._filter);
       }).map(login => login.guid);
     } else {
       matchingLoginGuids = this._logins.map(login => login.guid);
     }
 
     for (let listItem of this.shadowRoot.querySelectorAll("login-list-item")) {
+      if (!listItem.hasAttribute("guid")) {
+        // Don't hide the 'New Login' item if it is present.
+        continue;
+      }
       if (matchingLoginGuids.includes(listItem.getAttribute("guid"))) {
         if (listItem.hidden) {
           listItem.hidden = false;
         }
       } else if (!listItem.hidden) {
         listItem.hidden = true;
       }
     }
@@ -82,50 +102,49 @@ export default class LoginList extends R
         break;
       }
       case "AboutLoginsFilterLogins": {
         this._filter = event.detail.toLocaleLowerCase();
         this.render();
         break;
       }
       case "AboutLoginsLoginSelected": {
-        if (this._selectedGuid) {
-          if (this._selectedGuid == event.detail.guid) {
-            return;
-          }
-          let oldSelected = this.shadowRoot.querySelector(`login-list-item[guid="${this._selectedGuid}"]`);
-          oldSelected.classList.remove("selected");
+        if (this._selectedGuid == event.detail.guid) {
+          return;
         }
-        this._selectedGuid = event.detail.guid;
-        let newSelected = this.shadowRoot.querySelector(`login-list-item[guid="${event.detail.guid}"]`);
-        newSelected.classList.add("selected");
+
+        this._selectedGuid = event.detail.guid || null;
+        this.render();
         break;
       }
     }
   }
 
   static get reflectedFluentIDs() {
     return ["count",
             "last-used-option",
             "last-changed-option",
             "name-option",
+            "new-login-subtitle",
+            "new-login-title",
             "sort-label-text"];
   }
 
   static get observedAttributes() {
     return this.reflectedFluentIDs;
   }
 
-  clearSelection() {
-    if (!this._selectedGuid) {
-      return;
+  handleSpecialCaseFluentString(attrName) {
+    if (attrName != "new-login-subtitle" &&
+        attrName != "new-login-title") {
+      return false;
     }
-    let listItem = this.shadowRoot.querySelector(`login-list-item[guid="${this._selectedGuid}"]`);
-    listItem.classList.remove("selected");
-    this._selectedGuid = null;
+
+    this._blankLoginListItem.setAttribute(attrName, this.getAttribute(attrName));
+    return true;
   }
 
   setLogins(logins) {
     this._logins = logins;
     this.render();
   }
 
   loginAdded(login) {
--- a/browser/components/aboutlogins/tests/browser/browser_aaa_eventTelemetry_run_first.js
+++ b/browser/components/aboutlogins/tests/browser/browser_aaa_eventTelemetry_run_first.js
@@ -35,17 +35,17 @@ add_task(async function test_telemetry_e
     Services.telemetry.clearEvents();
     let events = Services.telemetry.snapshotEvents(
       Ci.nsITelemetry.DATASET_PRERELEASE_CHANNELS, true).content;
     return !events || !events.length;
   }, "Waiting for telemetry events to get cleared");
 
   await ContentTask.spawn(gBrowser.selectedBrowser, null, async function() {
     let loginList = content.document.querySelector("login-list");
-    let loginListItem = loginList.shadowRoot.querySelector("login-list-item");
+    let loginListItem = loginList.shadowRoot.querySelector("login-list-item[guid]");
     loginListItem.click();
   });
   await waitForTelemetryEventCount(1);
 
   await ContentTask.spawn(gBrowser.selectedBrowser, null, async function() {
     let loginItem = content.document.querySelector("login-item");
     let copyButton = loginItem.shadowRoot.querySelector(".copy-username-button");
     copyButton = copyButton.shadowRoot.querySelector(".copy-button");
@@ -99,17 +99,17 @@ add_task(async function test_telemetry_e
     let loginItem = content.document.querySelector("login-item");
     let cancelButton = loginItem.shadowRoot.querySelector(".cancel-button");
     cancelButton.click();
   });
   await waitForTelemetryEventCount(8);
 
   await ContentTask.spawn(gBrowser.selectedBrowser, null, async function() {
     let loginList = content.document.querySelector("login-list");
-    let loginListItem = loginList.shadowRoot.querySelector("login-list-item");
+    let loginListItem = loginList.shadowRoot.querySelector("login-list-item[guid]");
     loginListItem.click();
   });
   await waitForTelemetryEventCount(9);
 
   await ContentTask.spawn(gBrowser.selectedBrowser, null, async function() {
     let loginItem = content.document.querySelector("login-item");
     let deleteButton = loginItem.shadowRoot.querySelector(".delete-button");
     deleteButton.click();
--- a/browser/components/aboutlogins/tests/browser/browser_deleteLogin.js
+++ b/browser/components/aboutlogins/tests/browser/browser_deleteLogin.js
@@ -40,22 +40,23 @@ add_task(async function test_login_item(
   let browser = gBrowser.selectedBrowser;
   let deleteLoginMessageReceived = false;
   browser.messageManager.addMessageListener("AboutLogins:DeleteLogin", function onMsg() {
     deleteLoginMessageReceived = true;
     browser.messageManager.removeMessageListener("AboutLogins:DeleteLogin", onMsg);
   });
   await ContentTask.spawn(browser, gLogins, async (logins) => {
     let loginList = content.document.querySelector("login-list");
-    let loginListItems = loginList.shadowRoot.querySelectorAll("login-list-item");
-    loginListItems[0].click();
+    let loginListItem = loginList.shadowRoot.querySelector("login-list-item[guid]");
+    info("Clicking on the first login");
+    loginListItem.click();
 
     let loginItem = Cu.waiveXrays(content.document.querySelector("login-item"));
     let loginItemPopulated = await ContentTaskUtils.waitForCondition(() => {
-      return loginItem._login.guid == loginListItems[0].getAttribute("guid");
+      return loginItem._login.guid == loginListItem.getAttribute("guid");
     }, "Waiting for login item to get populated");
     ok(loginItemPopulated, "The login item should get populated");
 
     let deleteButton = loginItem.shadowRoot.querySelector(".delete-button");
     deleteButton.click();
   });
   ok(deleteLoginMessageReceived,
      "Clicking the delete button should send the AboutLogins:DeleteLogin messsage");
--- a/browser/components/aboutlogins/tests/browser/browser_openFiltered.js
+++ b/browser/components/aboutlogins/tests/browser/browser_openFiltered.js
@@ -4,22 +4,24 @@
 let nsLoginInfo = new Components.Constructor("@mozilla.org/login-manager/loginInfo;1",
                                              Ci.nsILoginInfo, "init");
 let TEST_LOGIN1 = new nsLoginInfo("https://example.com/", "https://example.com/", null, "user1", "pass1", "username", "password");
 let TEST_LOGIN2 = new nsLoginInfo("https://example2.com/", "https://example2.com/", null, "user2", "pass2", "username", "password");
 
 add_task(async function setup() {
   await SpecialPowers.pushPrefEnv({"set": [["signon.management.overrideURI", "about:logins?filter=%DOMAIN%"]] });
 
-  for (let login of [TEST_LOGIN1, TEST_LOGIN2]) {
-    let storageChangedPromised = TestUtils.topicObserved("passwordmgr-storage-changed",
-                                                         (_, data) => data == "addLogin");
-    login = Services.logins.addLogin(login);
-    await storageChangedPromised;
-  }
+  let storageChangedPromised = TestUtils.topicObserved("passwordmgr-storage-changed",
+                                                       (_, data) => data == "addLogin");
+  TEST_LOGIN1 = Services.logins.addLogin(TEST_LOGIN1);
+  await storageChangedPromised;
+  storageChangedPromised = TestUtils.topicObserved("passwordmgr-storage-changed",
+                                                   (_, data) => data == "addLogin");
+  TEST_LOGIN2 = Services.logins.addLogin(TEST_LOGIN2);
+  await storageChangedPromised;
   let tabOpenedPromise =
     BrowserTestUtils.waitForNewTab(gBrowser, "about:logins?filter=" + encodeURIComponent(TEST_LOGIN1.origin), true);
   LoginHelper.openPasswordManager(window, { filterString: TEST_LOGIN1.origin, entryPoint: "preferences" });
   await tabOpenedPromise;
   registerCleanupFunction(() => {
     BrowserTestUtils.removeTab(gBrowser.selectedTab);
     Services.logins.removeAllLogins();
   });
@@ -37,14 +39,15 @@ add_task(async function test_query_param
       return loginList._logins.length == 2;
     }, "Waiting for logins to be cached");
 
     let loginFilter = Cu.waiveXrays(content.document.querySelector("login-filter"));
     is(loginFilter.value, logins[0].origin, "The filter should be prepopulated");
 
     let hiddenLoginListItems = loginList.shadowRoot.querySelectorAll("login-list-item[hidden]");
     let visibleLoginListItems = loginList.shadowRoot.querySelectorAll("login-list-item:not([hidden])");
-    is(visibleLoginListItems.length, 1, "One login should be visible");
-    is(visibleLoginListItems[0].guid, logins[0].guid, "TEST_LOGIN1 should be visible");
+    is(visibleLoginListItems.length, 2, "The 'new' login and one login should be visible");
+    ok(!visibleLoginListItems[0].hasAttribute("guid"), "The 'new' login should be visible");
+    is(visibleLoginListItems[1].getAttribute("guid"), logins[0].guid, "TEST_LOGIN1 should be visible");
     is(hiddenLoginListItems.length, 1, "One login should be hidden");
-    is(hiddenLoginListItems[0].guid, logins[1].guid, "TEST_LOGIN2 should be hidden");
+    is(hiddenLoginListItems[0].getAttribute("guid"), logins[1].guid, "TEST_LOGIN2 should be hidden");
   });
 });
--- a/browser/components/aboutlogins/tests/browser/browser_updateLogin.js
+++ b/browser/components/aboutlogins/tests/browser/browser_updateLogin.js
@@ -29,17 +29,17 @@ add_task(async function test_show_logins
     ok(loginFound, "Stored logins should be displayed upon loading the page");
   });
 });
 
 add_task(async function test_login_item() {
   let browser = gBrowser.selectedBrowser;
   await ContentTask.spawn(browser, LoginHelper.loginToVanillaObject(TEST_LOGIN1), async (login) => {
     let loginList = content.document.querySelector("login-list");
-    let loginListItem = Cu.waiveXrays(loginList.shadowRoot.querySelector("login-list-item"));
+    let loginListItem = Cu.waiveXrays(loginList.shadowRoot.querySelector("login-list-item[guid]"));
     loginListItem.click();
 
     let loginItem = Cu.waiveXrays(content.document.querySelector("login-item"));
     let loginItemPopulated = await ContentTaskUtils.waitForCondition(() => {
       return loginItem._login.guid == loginListItem.getAttribute("guid") &&
              loginItem._login.guid == login.guid;
     }, "Waiting for login item to get populated");
     ok(loginItemPopulated, "The login item should get populated");
--- a/browser/components/aboutlogins/tests/chrome/test_login_list.html
+++ b/browser/components/aboutlogins/tests/chrome/test_login_list.html
@@ -61,28 +61,30 @@ add_task(async function setup() {
 
 add_task(async function test_empty_list() {
   ok(gLoginList, "loginList exists");
   is(gLoginList.textContent, "", "Initially empty");
 });
 
 add_task(async function test_populated_list() {
   gLoginList.setLogins([TEST_LOGIN_1, TEST_LOGIN_2]);
-  await asyncElementRendered();
   let loginListItems = gLoginList.shadowRoot.querySelectorAll("login-list-item");
-  is(loginListItems.length, 2, "Both logins should be displayed");
-  is(loginListItems[0].getAttribute("guid"), TEST_LOGIN_1.guid, "login-list-item should have correct guid attribute");
-  is(loginListItems[0].shadowRoot.querySelector(".origin").textContent, TEST_LOGIN_1.origin,
+  is(loginListItems.length, 3, "A blank login and the two stored logins should be displayed");
+  ok(!loginListItems[0].hasAttribute("guid"), "first login-list-item should be the 'new' item");
+  is(loginListItems[1].getAttribute("guid"), TEST_LOGIN_1.guid, "login-list-item should have correct guid attribute");
+  is(loginListItems[1].shadowRoot.querySelector(".origin").textContent, TEST_LOGIN_1.origin,
      "login-list-item origin should match");
-  is(loginListItems[0].shadowRoot.querySelector(".username").textContent, TEST_LOGIN_1.username,
+  is(loginListItems[1].shadowRoot.querySelector(".username").textContent, TEST_LOGIN_1.username,
      "login-list-item username should match");
-
-  ok(!loginListItems[0].classList.contains("selected"), "The first item should not be selected by default");
+  ok(loginListItems[0].classList.contains("selected"), "The first item should be selected by default");
   ok(!loginListItems[1].classList.contains("selected"), "The second item should not be selected by default");
-  loginListItems[0].click();
+  ok(!loginListItems[2].classList.contains("selected"), "The third item should not be selected by default");
+  loginListItems[1].click();
+  loginListItems = gLoginList.shadowRoot.querySelectorAll("login-list-item");
+  is(loginListItems.length, 2, "After selecting one, only the two stored logins should be displayed");
   ok(loginListItems[0].classList.contains("selected"), "The first item should be selected");
   ok(!loginListItems[1].classList.contains("selected"), "The second item should still not be selected");
 });
 
 add_task(async function test_filtered_list() {
   is(gLoginList.shadowRoot.querySelectorAll("login-list-item:not([hidden])").length, 2, "Both logins should be visible");
   let countSpan = gLoginList.shadowRoot.querySelector(".count");
   is(countSpan.textContent, "2", "Count should match full list length");
@@ -188,33 +190,42 @@ add_task(async function test_login_added
   is(loginListItems[2].getAttribute("guid"), newLogin.guid, "login-list-item3 should have correct guid attribute");
   ok(!loginListItems[0].hidden, "login-list-item1 should be visible");
   ok(loginListItems[1].hidden, "login-list-item2 should be hidden");
   ok(loginListItems[2].hidden, "login-list-item3 should be hidden");
   is(countSpan.textContent, "1", "Count should remain unchanged");
 });
 
 add_task(async function test_sorted_list() {
+  // Clear the selection so the 'new' login will be in the list too.
+  window.dispatchEvent(new CustomEvent("AboutLoginsLoginSelected", {
+    detail: {},
+  }));
+
   // sort by last used
   gLoginList.shadowRoot.getElementById("login-sort").selectedIndex = 1;
   let loginListItems = gLoginList.shadowRoot.querySelectorAll("login-list-item");
-  let timeUsed = loginListItems[0]._login.timeLastUsed;
-  let timeUsed2 = loginListItems[1]._login.timeLastUsed;
+  is(loginListItems.length, 4, "The list should contain the 'new' login and the three stored logins");
+  ok(!loginListItems[0]._login.guid, "The 'new' login should always be first (last used)");
+  let timeUsed = loginListItems[1]._login.timeLastUsed;
+  let timeUsed2 = loginListItems[2]._login.timeLastUsed;
   is(timeUsed2 > timeUsed, true, "Last used login should be displayed at top of list");
 
   // sort by name
   gLoginList.shadowRoot.getElementById("login-sort").selectedIndex = 0;
   loginListItems = gLoginList.shadowRoot.querySelectorAll("login-list-item");
-  let title = loginListItems[0]._login.title;
-  let title2 = loginListItems[1]._login.title;
+  ok(!loginListItems[0]._login.guid, "The 'new' login should always be first (name)");
+  let title = loginListItems[1]._login.title;
+  let title2 = loginListItems[2]._login.title;
   is(title.localeCompare(title2), -1, "Logins should be sorted alphabetically by hostname");
 
   // sort by last changed
   gLoginList.shadowRoot.getElementById("login-sort").selectedIndex = 2;
   loginListItems = gLoginList.shadowRoot.querySelectorAll("login-list-item");
-  let pwChanged = loginListItems[0]._login.timePasswordChanged;
-  let pwChanged2 = loginListItems[1]._login.timePasswordChanged;
+  ok(!loginListItems[0]._login.guid, "The 'new' login should always be first (last changed)");
+  let pwChanged = loginListItems[1]._login.timePasswordChanged;
+  let pwChanged2 = loginListItems[2]._login.timePasswordChanged;
   is(pwChanged2 > pwChanged, true, "Login with most recently changed password should be displayed at top of list");
 });
 </script>
 
 </body>
 </html>