Backed out changeset 500c7e81bdcc (bug 1626413) for browser_loginSortOrderRestored.js failures CLOSED TREE
authorBogdan Tara <btara@mozilla.com>
Thu, 02 Apr 2020 03:18:38 +0300
changeset 521708 7afd0c50200b0993e063c065f3b89c78afb19017
parent 521707 58a7405ae9cc56baff8587094962e0181e4fd714
child 521709 bd051fd71e1cb24626db992fe079109eca9aee6f
push id37274
push userdvarga@mozilla.com
push dateThu, 02 Apr 2020 09:51:45 +0000
treeherdermozilla-central@95ddb3213aec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1626413
milestone76.0a1
backs out500c7e81bdcc2b18e52b91b168eaabe4acac77fe
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
Backed out changeset 500c7e81bdcc (bug 1626413) for browser_loginSortOrderRestored.js failures CLOSED TREE
browser/components/aboutlogins/content/aboutLogins.html
browser/components/aboutlogins/content/components/login-list.js
browser/components/aboutlogins/tests/browser/browser_loginSortOrderRestored.js
--- a/browser/components/aboutlogins/content/aboutLogins.html
+++ b/browser/components/aboutlogins/content/aboutLogins.html
@@ -85,17 +85,17 @@
       <div class="meta">
         <label for="login-sort">
           <span data-l10n-id="login-list-sort-label-text"></span>
           <select id="login-sort">
             <option name="name" data-l10n-id="login-list-name-option" value="name">
             <option name="name-reverse" data-l10n-id="login-list-name-reverse-option" value="name-reverse">
             <option name="last-used" data-l10n-id="login-list-last-used-option" value="last-used">
             <option name="last-changed" data-l10n-id="login-list-last-changed-option" value="last-changed">
-            <option name="alerts" data-l10n-id="about-logins-login-list-alerts-option" value="alerts" hidden>
+            <option name="alerts" data-l10n-id="about-logins-login-list-alerts-option" value="alerts">
           </select>
         </label>
         <span class="count" data-l10n-id="login-list-count" data-l10n-args='{"count": 0}'></span>
       </div>
       <!-- This container is to work around bug 1569292 -->
       <div class="container">
         <ol role="listbox" tabindex="0" data-l10n-id="login-list"></ol>
         <div class="intro">
--- a/browser/components/aboutlogins/content/components/login-list.js
+++ b/browser/components/aboutlogins/content/components/login-list.js
@@ -7,31 +7,34 @@ import { recordTelemetryEvent } from "..
 
 const collator = new Intl.Collator();
 const sortFnOptions = {
   name: (a, b) => collator.compare(a.title, b.title),
   "name-reverse": (a, b) => collator.compare(b.title, a.title),
   "last-used": (a, b) => a.timeLastUsed < b.timeLastUsed,
   "last-changed": (a, b) => a.timePasswordChanged < b.timePasswordChanged,
   alerts: (a, b, breachesByLoginGUID, vulnerableLoginsByLoginGUID) => {
+    if (!breachesByLoginGUID && !vulnerableLoginsByLoginGUID) {
+      return 0;
+    }
     const aIsBreached = breachesByLoginGUID && breachesByLoginGUID.has(a.guid);
     const bIsBreached = breachesByLoginGUID && breachesByLoginGUID.has(b.guid);
     const aIsVulnerable =
       vulnerableLoginsByLoginGUID && vulnerableLoginsByLoginGUID.has(a.guid);
     const bIsVulnerable =
       vulnerableLoginsByLoginGUID && vulnerableLoginsByLoginGUID.has(b.guid);
 
     if ((aIsBreached && !bIsBreached) || (aIsVulnerable && !bIsVulnerable)) {
       return -1;
     }
 
     if ((!aIsBreached && bIsBreached) || (!aIsVulnerable && bIsVulnerable)) {
       return 1;
     }
-    return sortFnOptions.name(a, b);
+    return collator.compare(a.title, b.title);
   },
 };
 
 export default class LoginList extends HTMLElement {
   constructor() {
     super();
     // An array of login GUIDs, stored in sorted order.
     this._loginGuidsSortedOrder = [];
@@ -421,24 +424,16 @@ export default class LoginList extends H
     this._internalSetMonitorData(
       internalMemberName,
       this[internalMemberName],
       false
     );
   }
 
   setSortDirection(sortDirection) {
-    // The 'alerts' sort becomes visible when there are known alerts.
-    // Don't restore to the 'alerts' sort if there are no alerts to show.
-    if (
-      sortDirection == "alerts" &&
-      this._sortSelect.namedItem("alerts").hidden
-    ) {
-      return;
-    }
     this._sortSelect.value = sortDirection;
     this._applySortAndScrollToTop();
     this._selectFirstVisibleLogin();
   }
 
   /**
    * @param {login} login A login that was added to storage.
    */
@@ -544,22 +539,25 @@ export default class LoginList extends H
     return matchingLoginGuids;
   }
 
   _applySort() {
     const sort = this._sortSelect.value;
     this._loginGuidsSortedOrder = this._loginGuidsSortedOrder.sort((a, b) => {
       let loginA = this._logins[a].login;
       let loginB = this._logins[b].login;
-      return sortFnOptions[sort](
-        loginA,
-        loginB,
-        this._breachesByLoginGUID,
-        this._vulnerableLoginsByLoginGUID
-      );
+      if (this._breachesByLoginGUID && this._vulnerableLoginsByLoginGUID) {
+        return sortFnOptions[sort](
+          loginA,
+          loginB,
+          this._breachesByLoginGUID,
+          this._vulnerableLoginsByLoginGUID
+        );
+      }
+      return sortFnOptions[sort](loginA, loginB);
     });
   }
 
   _applySortAndScrollToTop() {
     this._applySort();
     this.render();
     this._list.scrollTop = 0;
   }
--- a/browser/components/aboutlogins/tests/browser/browser_loginSortOrderRestored.js
+++ b/browser/components/aboutlogins/tests/browser/browser_loginSortOrderRestored.js
@@ -1,66 +1,51 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 ChromeUtils.import("resource://testing-common/TelemetryTestUtils.jsm", this);
 ChromeUtils.import("resource://testing-common/LoginTestUtils.jsm", this);
 
-EXPECTED_BREACH = {
-  AddedDate: "2018-12-20T23:56:26Z",
-  BreachDate: "2018-12-16",
-  Domain: "breached.example.com",
-  Name: "Breached",
-  PwnCount: 1643100,
-  DataClasses: ["Email addresses", "Usernames", "Passwords", "IP addresses"],
-  _status: "synced",
-  id: "047940fe-d2fd-4314-b636-b4a952ee0043",
-  last_modified: "1541615610052",
-  schema: "1541615609018",
-};
-
-const SORT_PREF_NAME = "signon.management.page.sort";
-
 add_task(async function setup() {
-  TEST_LOGIN3.QueryInterface(Ci.nsILoginMetaInfo).timePasswordChanged = 1;
+  TEST_LOGIN2.QueryInterface(Ci.nsILoginMetaInfo).timePasswordChanged = 1;
   TEST_LOGIN1 = await addLogin(TEST_LOGIN1);
   info(`TEST_LOGIN1 added with guid=${TEST_LOGIN1.guid}`);
-  TEST_LOGIN3 = await addLogin(TEST_LOGIN3);
-  info(`TEST_LOGIN3 added with guid=${TEST_LOGIN3.guid}`);
+  TEST_LOGIN2 = await addLogin(TEST_LOGIN2);
+  info(`TEST_LOGIN2 added with guid=${TEST_LOGIN2.guid}`);
   registerCleanupFunction(() => {
     Services.logins.removeAllLogins();
-    Services.prefs.clearUserPref(SORT_PREF_NAME);
+    Services.prefs.clearUserPref("signon.management.page.sort");
   });
 });
 
 add_task(async function test_sort_order_persisted() {
   await BrowserTestUtils.withNewTab(
     {
       gBrowser,
       url: "about:logins",
     },
     async function(browser) {
       await ContentTask.spawn(
         browser,
-        [TEST_LOGIN1.guid, TEST_LOGIN3.guid],
-        async function([testLogin1Guid, testLogin3Guid]) {
+        [TEST_LOGIN1.guid, TEST_LOGIN2.guid],
+        async function([testLogin1Guid, testLogin2Guid]) {
           let loginList = Cu.waiveXrays(
             content.document.querySelector("login-list")
           );
           is(
             loginList._sortSelect.value,
-            "alerts",
-            "selected sort should be 'alerts' since there is a breached login"
+            "name",
+            "default selected sort should be 'name'"
           );
           is(
             loginList._list.querySelector(
               ".login-list-item[data-guid]:not([hidden])"
             ).dataset.guid,
-            testLogin3Guid,
-            "the first login should be TEST_LOGIN3 since they are sorted by alerts"
+            testLogin2Guid,
+            "the first login should be TEST_LOGIN2 since they are sorted by origin"
           );
 
           loginList._sortSelect.value = "last-changed";
           loginList._sortSelect.dispatchEvent(
             new content.Event("change", { bubbles: true })
           );
           is(
             loginList._list.querySelector(
@@ -69,87 +54,36 @@ add_task(async function test_sort_order_
             testLogin1Guid,
             "the first login should be TEST_LOGIN1 since it has the most recent timePasswordChanged value"
           );
         }
       );
     }
   );
 
-  is(
-    Services.prefs.getCharPref(SORT_PREF_NAME),
-    "last-changed",
-    "'last-changed' should be stored in the pref"
-  );
-
-  // Set the pref to the value used in Fx70-76 to confirm our
-  // backwards-compat support that "breached" is changed to "alerts"
-  Services.prefs.setCharPref(SORT_PREF_NAME, "breached");
   await BrowserTestUtils.withNewTab(
     {
       gBrowser,
       url: "about:logins",
     },
     async function(browser) {
-      await ContentTask.spawn(browser, TEST_LOGIN3.guid, async function(
-        testLogin3Guid
+      await ContentTask.spawn(browser, TEST_LOGIN1.guid, async function(
+        testLogin1Guid
       ) {
         let loginList = Cu.waiveXrays(
           content.document.querySelector("login-list")
         );
         is(
           loginList._sortSelect.value,
-          "alerts",
-          "selected sort should be restored to 'alerts' since 'breached' was in prefs"
+          "last-changed",
+          "selected sort should be restored to 'last-changed'"
         );
         is(
           loginList._list.querySelector(
             ".login-list-item[data-guid]:not([hidden])"
           ).dataset.guid,
-          testLogin3Guid,
-          "the first login should be TEST_LOGIN3 since they are sorted by alerts"
-        );
-      });
-    }
-  );
-
-  let storageChangedPromised = TestUtils.topicObserved(
-    "passwordmgr-storage-changed",
-    (_, data) => data == "removeLogin"
-  );
-  Services.logins.removeLogin(TEST_LOGIN3);
-  await storageChangedPromised;
-  TEST_LOGIN2 = await addLogin(TEST_LOGIN2);
-
-  is(
-    Services.prefs.getCharPref(SORT_PREF_NAME),
-    "breached",
-    "confirm that the stored sort is still 'breached' and as such shouldn't apply when the page loads"
-  );
-  await BrowserTestUtils.withNewTab(
-    {
-      gBrowser,
-      url: "about:logins",
-    },
-    async function(browser) {
-      await ContentTask.spawn(browser, TEST_LOGIN2.guid, async function(
-        testLogin2Guid
-      ) {
-        let loginList = Cu.waiveXrays(
-          content.document.querySelector("login-list")
-        );
-        is(
-          loginList._sortSelect.value,
-          "name",
-          "selected sort should be name since 'alerts' no longer applies with no breached or vulnerable logins"
-        );
-        info(loginList._list.innerHTML);
-        is(
-          loginList._list.querySelector(
-            ".login-list-item[data-guid]:not([hidden])"
-          ).dataset.guid,
-          testLogin2Guid,
-          "the first login should be TEST_LOGIN2 since it is sorted first by 'name'"
+          testLogin1Guid,
+          "the first login should still be TEST_LOGIN1 since it has the most recent timePasswordChanged value"
         );
       });
     }
   );
 });