From 09dd90f42e9b17af0796819c8581d0fe9cbcfa41 Mon Sep 17 00:00:00 2001
From: Antonin LOUBIERE <pyjacpp@laposte.net>
Date: Fri, 13 Aug 2021 15:16:55 +0000
Subject: [PATCH] Bug 1592682 - Add Section Headers in login list for date and
 alert sorts. r=jaws,tgiles,fluent-reviewers

Add Section Headers in login list (about:logins) for date and alert sorts.

Differential Revision: https://phabricator.services.mozilla.com/D120788
---
 .../aboutlogins/content/aboutLogins.html      |   7 +
 .../content/components/login-list-section.js  |  34 ++++
 .../content/components/login-list.css         |  13 ++
 .../content/components/login-list.js          | 188 ++++++++++++++++--
 browser/components/aboutlogins/jar.mn         |   1 +
 .../tests/chrome/test_login_list.html         |  32 +--
 browser/locales/en-US/browser/aboutLogins.ftl |   6 +
 7 files changed, 250 insertions(+), 31 deletions(-)
 create mode 100644 browser/components/aboutlogins/content/components/login-list-section.js

diff --git a/browser/components/aboutlogins/content/aboutLogins.html b/browser/components/aboutlogins/content/aboutLogins.html
index 16d905f28a5d..8b84582f5a28 100644
--- a/browser/components/aboutlogins/content/aboutLogins.html
+++ b/browser/components/aboutlogins/content/aboutLogins.html
@@ -23,6 +23,7 @@
     <script type="module" src="chrome://browser/content/aboutlogins/components/login-item.js"></script>
     <script type="module" src="chrome://browser/content/aboutlogins/components/login-list.js"></script>
     <script type="module" src="chrome://browser/content/aboutlogins/components/login-list-item.js"></script>
+    <script type="module" src="chrome://browser/content/aboutlogins/components/login-list-section.js"></script>
     <script type="module" src="chrome://browser/content/aboutlogins/components/menu-button.js"></script>
     <script type="module" src="chrome://browser/content/aboutlogins/aboutLogins.js"></script>
     <link rel="stylesheet" href="chrome://global/skin/in-content/common.css">
@@ -223,6 +224,12 @@
       </li>
     </template>
 
+    <template id="login-list-section-template">
+      <section class="login-list-section">
+        <span class="login-list-header" dir="auto"></span>
+      </section>
+    </template>
+
     <template id="login-intro-template">
       <link rel="stylesheet" href="chrome://global/skin/in-content/common.css">
       <link rel="stylesheet" href="chrome://browser/content/aboutlogins/common.css">
diff --git a/browser/components/aboutlogins/content/components/login-list-section.js b/browser/components/aboutlogins/content/components/login-list-section.js
new file mode 100644
index 000000000000..5495f55e28a5
--- /dev/null
+++ b/browser/components/aboutlogins/content/components/login-list-section.js
@@ -0,0 +1,34 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+export default class LoginListHeaderFactory {
+  static ID_PREFIX = "id-";
+
+  static create(header) {
+    let template = document.querySelector("#login-list-section-template");
+    let fragment = template.content.cloneNode(true);
+    let sectionItem = fragment.firstElementChild;
+
+    this.update(sectionItem, header);
+
+    return sectionItem;
+  }
+
+  static update(headerItem, header) {
+    let headerElement = headerItem.querySelector(".login-list-header");
+    if (header) {
+      if (header.startsWith(this.ID_PREFIX)) {
+        document.l10n.setAttributes(
+          headerElement,
+          header.substring(this.ID_PREFIX.length)
+        );
+      } else {
+        headerElement.textContent = header;
+      }
+      headerElement.hidden = false;
+    } else {
+      headerElement.hidden = true;
+    }
+  }
+}
diff --git a/browser/components/aboutlogins/content/components/login-list.css b/browser/components/aboutlogins/content/components/login-list.css
index b6d528fd0644..36443d40099e 100644
--- a/browser/components/aboutlogins/content/components/login-list.css
+++ b/browser/components/aboutlogins/content/components/login-list.css
@@ -86,6 +86,7 @@ ol {
   padding-inline-start: 0;
   overflow: hidden auto;
   flex-grow: 1;
+  scroll-padding-top: 24px; /* there is the section header that is sticky to the top */
 }
 
 .create-login-button {
@@ -107,6 +108,18 @@ ol {
   user-select: none;
 }
 
+.login-list-header {
+  display: block;
+  position: sticky;
+  top: 0;
+  font-size: .85em;
+  font-weight: 600;
+  padding: 4px 16px;
+  border-bottom: 1px solid var(--in-content-border-color);
+  background-color: var(--in-content-box-background);
+}
+
+:not([hidden]) + .login-list-section,
 .login-list-item + .login-list-item {
   border-top: 1px solid var(--in-content-border-color);
 }
diff --git a/browser/components/aboutlogins/content/components/login-list.js b/browser/components/aboutlogins/content/components/login-list.js
index c518ccd4eca1..7e04c60f8fca 100644
--- a/browser/components/aboutlogins/content/components/login-list.js
+++ b/browser/components/aboutlogins/content/components/login-list.js
@@ -3,9 +3,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import LoginListItemFactory from "./login-list-item.js";
+import LoginListSectionFactory from "./login-list-section.js";
 import { recordTelemetryEvent } from "../aboutLoginsUtils.js";
 
 const collator = new Intl.Collator();
+const monthFormatter = new Intl.DateTimeFormat(undefined, { month: "long" });
+const yearMonthFormatter = new Intl.DateTimeFormat(undefined, {
+  year: "numeric",
+  month: "long",
+});
+const dayDuration = 24 * 60 * 60_000;
 const sortFnOptions = {
   name: (a, b) => collator.compare(a.title, b.title),
   "name-reverse": (a, b) => collator.compare(b.title, a.title),
@@ -32,6 +39,60 @@ const sortFnOptions = {
   },
 };
 
+const headersFnOptions = {
+  // TODO: name should use the ICU API, see Bug 1592834
+  // name: l =>
+  //   l.title.length && letterRegExp.test(l.title[0])
+  //     ? l.title[0].toUpperCase()
+  //     : "#",
+  // "name-reverse": l => headersFnOptions.name(l),
+  name: () => "",
+  "name-reverse": () => "",
+  "last-used": l => headerFromDate(l.timeLastUsed),
+  "last-changed": l => headerFromDate(l.timePasswordChanged),
+  alerts: (l, breachesByLoginGUID, vulnerableLoginsByLoginGUID) => {
+    const isBreached = breachesByLoginGUID && breachesByLoginGUID.has(l.guid);
+    const isVulnerable =
+      vulnerableLoginsByLoginGUID && vulnerableLoginsByLoginGUID.has(l.guid);
+    if (isBreached) {
+      return (
+        LoginListSectionFactory.ID_PREFIX + "about-logins-list-section-breach"
+      );
+    } else if (isVulnerable) {
+      return (
+        LoginListSectionFactory.ID_PREFIX +
+        "about-logins-list-section-vulnerable"
+      );
+    }
+    return (
+      LoginListSectionFactory.ID_PREFIX + "about-logins-list-section-nothing"
+    );
+  },
+};
+
+function headerFromDate(timestamp) {
+  let now = new Date();
+  now.setHours(0, 0, 0, 0); // reset to start of day
+  let date = new Date(timestamp);
+
+  if (now < date) {
+    return (
+      LoginListSectionFactory.ID_PREFIX + "about-logins-list-section-today"
+    );
+  } else if (now - dayDuration < date) {
+    return (
+      LoginListSectionFactory.ID_PREFIX + "about-logins-list-section-yesterday"
+    );
+  } else if (now - 7 * dayDuration < date) {
+    return LoginListSectionFactory.ID_PREFIX + "about-logins-list-section-week";
+  } else if (now.getFullYear() == date.getFullYear()) {
+    return monthFormatter.format(date);
+  } else if (now.getFullYear() - 1 == date.getFullYear()) {
+    return yearMonthFormatter.format(date);
+  }
+  return String(date.getFullYear());
+}
+
 export default class LoginList extends HTMLElement {
   constructor() {
     super();
@@ -39,6 +100,8 @@ export default class LoginList extends HTMLElement {
     this._loginGuidsSortedOrder = [];
     // A map of login GUID -> {login, listItem}.
     this._logins = {};
+    // A map of section header -> sectionItem
+    this._sections = {};
     this._filter = "";
     this._selectedGuid = null;
     this._blankLoginListItem = LoginListItemFactory.create({});
@@ -128,14 +191,40 @@ export default class LoginList extends HTMLElement {
       listItem.hidden = !visibleLoginGuids.has(listItem.dataset.guid);
     }
 
-    // Re-arrange the login-list-items according to their sort
-    for (let i = this._loginGuidsSortedOrder.length - 1; i >= 0; i--) {
-      let guid = this._loginGuidsSortedOrder[i];
-      let { listItem } = this._logins[guid];
-      this._list.insertBefore(
-        listItem,
-        this._blankLoginListItem.nextElementSibling
-      );
+    let sectionsKey = Object.keys(this._sections);
+    for (let sectionKey of sectionsKey) {
+      this._sections[sectionKey]._inUse = false;
+    }
+
+    if (this._loginGuidsSortedOrder.length) {
+      let section = null;
+      let currentHeader = null;
+      // Re-arrange the login-list-items according to their sort and
+      // create / re-arrange sections
+      for (let i = this._loginGuidsSortedOrder.length - 1; i >= 0; i--) {
+        let guid = this._loginGuidsSortedOrder[i];
+        let { listItem, _header } = this._logins[guid];
+
+        if (!listItem.hidden) {
+          if (currentHeader != _header) {
+            section = this.renderSectionHeader((currentHeader = _header));
+          }
+
+          section.insertBefore(
+            listItem,
+            section.firstElementChild.nextElementSibling
+          );
+        }
+      }
+    }
+
+    for (let sectionKey of sectionsKey) {
+      let section = this._sections[sectionKey];
+      if (section._inUse) {
+        continue;
+      }
+
+      section.hidden = true;
     }
 
     let activeDescendantId = this._list.getAttribute("aria-activedescendant");
@@ -167,6 +256,22 @@ export default class LoginList extends HTMLElement {
     }
   }
 
+  renderSectionHeader(header) {
+    let section = this._sections[header];
+    if (!section) {
+      section = this._sections[header] = LoginListSectionFactory.create(header);
+    }
+
+    this._list.insertBefore(
+      section,
+      this._blankLoginListItem.nextElementSibling
+    );
+
+    section._inUse = true;
+    section.hidden = false;
+    return section;
+  }
+
   handleEvent(event) {
     switch (event.type) {
       case "click": {
@@ -209,6 +314,7 @@ export default class LoginList extends HTMLElement {
         break;
       }
       case "change": {
+        this._applyHeaders();
         this._applySortAndScrollToTop();
         const extra = { sort_key: this._sortSelect.value };
         recordTelemetryEvent({ object: "list", method: "sort", extra });
@@ -332,6 +438,8 @@ export default class LoginList extends HTMLElement {
       map[login.guid] = { login };
       return map;
     }, {});
+    this._sections = {};
+    this._applyHeaders();
     this._applySort();
     this._list.textContent = "";
     this._list.appendChild(this._blankLoginListItem);
@@ -420,6 +528,7 @@ export default class LoginList extends HTMLElement {
         const alertsSortOptionElement = this._sortSelect.namedItem("alerts");
         alertsSortOptionElement.hidden = false;
         this._sortSelect.selectedIndex = alertsSortOptionElement.index;
+        this._applyHeaders();
         this._applySortAndScrollToTop();
         this._selectFirstVisibleLogin();
       }
@@ -455,6 +564,7 @@ export default class LoginList extends HTMLElement {
       return;
     }
     this._sortSelect.value = sortDirection;
+    this._applyHeaders();
     this._applySortAndScrollToTop();
     this._selectFirstVisibleLogin();
   }
@@ -465,6 +575,7 @@ export default class LoginList extends HTMLElement {
   loginAdded(login) {
     this._logins[login.guid] = { login };
     this._loginGuidsSortedOrder.push(login.guid);
+    this._applyHeaders(false);
     this._applySort();
 
     // Add the list item and update any other related state that may pertain
@@ -486,10 +597,12 @@ export default class LoginList extends HTMLElement {
   loginModified(login) {
     this._logins[login.guid] = Object.assign(this._logins[login.guid], {
       login,
+      _header: null, // reset header
     });
+    this._applyHeaders(false);
     this._applySort();
-    let { listItem } = this._logins[login.guid];
-    LoginListItemFactory.update(listItem, login);
+    let loginObject = this._logins[login.guid];
+    LoginListItemFactory.update(loginObject.listItem, login);
 
     // Update any other related state that may pertain to the list item
     // such as breach alerts that may or may not now apply.
@@ -577,6 +690,20 @@ export default class LoginList extends HTMLElement {
     });
   }
 
+  _applyHeaders(updateAll = true) {
+    let headerFn = headersFnOptions[this._sortSelect.value];
+    for (let guid of this._loginGuidsSortedOrder) {
+      let login = this._logins[guid];
+      if (updateAll || !login._header) {
+        login._header = headerFn(
+          login.login,
+          this._breachesByLoginGUID,
+          this._vulnerableLoginsByLoginGUID
+        );
+      }
+    }
+  }
+
   _applySortAndScrollToTop() {
     this._applySort();
     this.render();
@@ -631,20 +758,43 @@ export default class LoginList extends HTMLElement {
     let activeDescendantId = this._list.getAttribute("aria-activedescendant");
     let activeDescendant =
       activeDescendantId && this.shadowRoot.getElementById(activeDescendantId);
-    if (!activeDescendant || activeDescendant.hidden) {
+    if (
+      !activeDescendant ||
+      activeDescendant.hidden ||
+      !activeDescendant.classList.contains("login-list-item")
+    ) {
       activeDescendant =
         this._list.querySelector(".login-list-item[data-guid]:not([hidden])") ||
         this._list.firstElementChild;
     }
     let newlyFocusedItem = null;
-    let previousItem = activeDescendant.previousElementSibling;
-    while (previousItem && previousItem.hidden) {
-      previousItem = previousItem.previousElementSibling;
-    }
-    let nextItem = activeDescendant.nextElementSibling;
-    while (nextItem && nextItem.hidden) {
-      nextItem = nextItem.nextElementSibling;
-    }
+    let previousItem = activeDescendant;
+    do {
+      previousItem =
+        (previousItem.tagName == "SECTION"
+          ? previousItem.lastElementChild
+          : previousItem.previousElementSibling) ||
+        (previousItem.parentElement.tagName == "SECTION" &&
+          previousItem.parentElement.previousElementSibling);
+    } while (
+      previousItem &&
+      (previousItem.hidden ||
+        !previousItem.classList.contains("login-list-item"))
+    );
+
+    let nextItem = activeDescendant;
+    do {
+      nextItem =
+        (nextItem.tagName == "SECTION"
+          ? nextItem.firstElementChild.nextElementSibling
+          : nextItem.nextElementSibling) ||
+        (nextItem.parentElement.tagName == "SECTION" &&
+          nextItem.parentElement.nextElementSibling);
+    } while (
+      nextItem &&
+      (nextItem.hidden || !nextItem.classList.contains("login-list-item"))
+    );
+
     if (event.type == "keydown") {
       switch (event.key) {
         case "ArrowDown": {
diff --git a/browser/components/aboutlogins/jar.mn b/browser/components/aboutlogins/jar.mn
index 58759463b3d9..fa5632000ecf 100644
--- a/browser/components/aboutlogins/jar.mn
+++ b/browser/components/aboutlogins/jar.mn
@@ -25,6 +25,7 @@ browser.jar:
   content/browser/aboutlogins/components/login-list.css                  (content/components/login-list.css)
   content/browser/aboutlogins/components/login-list.js                   (content/components/login-list.js)
   content/browser/aboutlogins/components/login-list-item.js              (content/components/login-list-item.js)
+  content/browser/aboutlogins/components/login-list-section.js           (content/components/login-list-section.js)
   content/browser/aboutlogins/components/menu-button.css                 (content/components/menu-button.css)
   content/browser/aboutlogins/components/menu-button.js                  (content/components/menu-button.js)
   content/browser/aboutlogins/icons/breached-website.svg                 (content/icons/breached-website.svg)
diff --git a/browser/components/aboutlogins/tests/chrome/test_login_list.html b/browser/components/aboutlogins/tests/chrome/test_login_list.html
index f302c21d5224..ff65c5fbb558 100644
--- a/browser/components/aboutlogins/tests/chrome/test_login_list.html
+++ b/browser/components/aboutlogins/tests/chrome/test_login_list.html
@@ -272,6 +272,15 @@ add_task(async function test_breach_indicator() {
 });
 
 add_task(async function test_filtered_list() {
+  function findItemFromUsername(list, username) {
+    for (let item of list) {
+      if ((item._cachedUsername || (item._cachedUsername = item.querySelector('.username').textContent)) == username) {
+        return item;
+      }
+    }
+    ok(false, `The ${username} wasn't in the list of logins.`)
+    return list[0];
+  }
   gLoginList.setLogins([TEST_LOGIN_1, TEST_LOGIN_2]);
   let emptySearchText = gLoginList.shadowRoot.querySelector(".empty-search-message");
   ok(isHidden(emptySearchText), "The empty search text should be hidden when there are results in the list");
@@ -295,8 +304,8 @@ add_task(async function test_filtered_list() {
   is(JSON.parse(countSpan.getAttribute("data-l10n-args")).count, 1, "Count should match result amount");
   ok(isHidden(emptySearchText), "The empty search text should be hidden when there are results in the list");
   loginListItems = gLoginList.shadowRoot.querySelectorAll(".login-list-item[data-guid]");
-  ok(loginListItems[0].hidden, "user1 should be hidden");
-  ok(!loginListItems[1].hidden, "user2 should be visible");
+  ok(findItemFromUsername(loginListItems, 'user1').hidden, "user1 should be hidden");
+  ok(!findItemFromUsername(loginListItems, 'user2').hidden, "user2 should be visible");
   window.dispatchEvent(new CustomEvent("AboutLoginsFilterLogins", {
     bubbles: true,
     detail: "user",
@@ -305,8 +314,8 @@ add_task(async function test_filtered_list() {
   ok(!gLoginList._sortSelect.disabled, "The sort should be enabled when there are visible logins in the list");
   ok(isHidden(emptySearchText), "The empty search text should be hidden when there are results in the list");
   loginListItems = gLoginList.shadowRoot.querySelectorAll(".login-list-item[data-guid]");
-  ok(!loginListItems[0].hidden, "user1 should be visible");
-  ok(!loginListItems[1].hidden, "user2 should be visible");
+  ok(!findItemFromUsername(loginListItems, 'user1').hidden, "user1 should be visible");
+  ok(!findItemFromUsername(loginListItems, 'user2').hidden, "user2 should be visible");
   window.dispatchEvent(new CustomEvent("AboutLoginsFilterLogins", {
     bubbles: true,
     detail: "foo",
@@ -316,8 +325,8 @@ add_task(async function test_filtered_list() {
   ok(!isHidden(emptySearchText), "The empty search text should be visible when there are no results in the list");
   isnot(gLoginList.shadowRoot.querySelector(".container > ol").getAttribute("aria-activedescendant"), "new-login-list-item", "new-login-list-item shouldn't be the active descendant");
   loginListItems = gLoginList.shadowRoot.querySelectorAll(".login-list-item[data-guid]");
-  ok(loginListItems[0].hidden, "user1 should be hidden");
-  ok(loginListItems[1].hidden, "user2 should be hidden");
+  ok(findItemFromUsername(loginListItems, 'user1').hidden, "user1 should be hidden");
+  ok(findItemFromUsername(loginListItems, 'user2').hidden, "user2 should be hidden");
   window.dispatchEvent(new CustomEvent("AboutLoginsFilterLogins", {
     bubbles: true,
     detail: "",
@@ -326,8 +335,8 @@ add_task(async function test_filtered_list() {
   ok(isHidden(emptySearchText), "The empty search text should be hidden when there are results in the list");
   is(JSON.parse(countSpan.getAttribute("data-l10n-args")).count, 2, "Count should be reset to full list length");
   loginListItems = gLoginList.shadowRoot.querySelectorAll(".login-list-item[data-guid]");
-  ok(!loginListItems[0].hidden, "user1 should be visible");
-  ok(!loginListItems[1].hidden, "user2 should be visible");
+  ok(!findItemFromUsername(loginListItems, 'user1').hidden, "user1 should be visible");
+  ok(!findItemFromUsername(loginListItems, 'user2').hidden, "user2 should be visible");
 
   info("Add an HTTP Auth login");
   gLoginList.setLogins([TEST_LOGIN_1, TEST_LOGIN_2, TEST_HTTP_AUTH_LOGIN_1]);
@@ -340,10 +349,9 @@ add_task(async function test_filtered_list() {
   }));
   is(JSON.parse(countSpan.getAttribute("data-l10n-args")).count, 1, "Count should match result amount");
   loginListItems = gLoginList.shadowRoot.querySelectorAll(".login-list-item[data-guid]");
-  ok(loginListItems[0].hidden, "user1 should be hidden");
-  ok(loginListItems[1].hidden, "user2 should be hidden");
-  ok(!loginListItems[2].hidden, "http_auth_user should be visible");
-  is(loginListItems[2].querySelector(".username").textContent, "http_auth_user", "Verify the login with a matching httpRealm is visible");
+  ok(findItemFromUsername(loginListItems, 'user1').hidden, "user1 should be hidden");
+  ok(findItemFromUsername(loginListItems, 'user2').hidden, "user2 should be hidden");
+  ok(!findItemFromUsername(loginListItems, 'http_auth_user').hidden, "http_auth_user should be visible");
 
   gLoginList.setLogins([TEST_LOGIN_1, TEST_LOGIN_2]);
   window.dispatchEvent(new CustomEvent("AboutLoginsFilterLogins", {
diff --git a/browser/locales/en-US/browser/aboutLogins.ftl b/browser/locales/en-US/browser/aboutLogins.ftl
index 1af390c15ee2..50b18af99ac7 100644
--- a/browser/locales/en-US/browser/aboutLogins.ftl
+++ b/browser/locales/en-US/browser/aboutLogins.ftl
@@ -59,6 +59,12 @@ about-logins-list-item-breach-icon =
   .title = Breached website
 about-logins-list-item-vulnerable-password-icon =
   .title = Vulnerable password
+about-logins-list-section-breach = Breached websites
+about-logins-list-section-vulnerable = Vulnerable passwords
+about-logins-list-section-nothing = No alert
+about-logins-list-section-today = Today
+about-logins-list-section-yesterday = Yesterday
+about-logins-list-section-week = Last 7 days
 
 ## Introduction screen