Bug 1507200 - Sort themes with previews higher in about:addons r=aswan
authorMark Striemer <mstriemer@mozilla.com>
Tue, 04 Dec 2018 20:01:17 +0000
changeset 505931 fa0a79862921
parent 505930 e743bb11887d
child 505932 4ee2c005d4ff
push id10301
push userarchaeopteryx@coole-files.de
push dateThu, 06 Dec 2018 16:36:14 +0000
treeherdermozilla-beta@7d2f3c71997c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1507200
milestone65.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 1507200 - Sort themes with previews higher in about:addons r=aswan Differential Revision: https://phabricator.services.mozilla.com/D12120
toolkit/mozapps/extensions/content/extensions.js
toolkit/mozapps/extensions/content/extensions.xml
toolkit/mozapps/extensions/test/browser/browser_theme_previews.js
--- a/toolkit/mozapps/extensions/content/extensions.js
+++ b/toolkit/mozapps/extensions/content/extensions.js
@@ -1563,16 +1563,25 @@ function sortElements(aElements, aSortBy
     // If we're in descending order, swap a and b, because
     // we don't ever want to have descending uiStates
     if (!aAscending)
       [a, b] = [b, a];
 
     return (UISTATE_ORDER.indexOf(a) - UISTATE_ORDER.indexOf(b));
   }
 
+  // Prioritize themes that have screenshots.
+  function hasPreview(aHasStr, bHasStr) {
+    let aHas = aHasStr == "true";
+    let bHas = bHasStr == "true";
+    if (aHas == bHas)
+      return 0;
+    return aHas ? -1 : 1;
+  }
+
   function getValue(aObj, aKey) {
     if (!aObj)
       return null;
 
     if (aObj.hasAttribute(aKey))
       return aObj.getAttribute(aKey);
 
     var addon = aObj.mAddon || aObj.mInstall;
@@ -1609,16 +1618,18 @@ function sortElements(aElements, aSortBy
     aSortFuncs[i] = stringCompare;
 
     if (sortBy == "uiState")
       aSortFuncs[i] = uiStateCompare;
     else if (DATE_FIELDS.includes(sortBy))
       aSortFuncs[i] = dateCompare;
     else if (NUMERIC_FIELDS.includes(sortBy))
       aSortFuncs[i] = numberCompare;
+    else if (sortBy == "hasPreview")
+      aSortFuncs[i] = hasPreview;
   }
 
 
   aElements.sort(function(a, b) {
     if (!aAscending)
       [a, b] = [b, a];
 
     for (let i = 0; i < aSortFuncs.length; i++) {
@@ -1630,18 +1641,19 @@ function sortElements(aElements, aSortBy
         return 0;
       if (!aValue)
         return -1;
       if (!bValue)
         return 1;
       if (aValue != bValue) {
         var result = aSortFuncs[i](aValue, bValue);
 
-        if (result != 0)
+        if (result != 0) {
           return result;
+        }
       }
     }
 
     // If we got here, then all values of a and b
     // must have been equal.
     return 0;
 
   });
@@ -2419,17 +2431,23 @@ var gListView = {
       for (let addonItem of aAddonsList)
         elements.push(createItem(addonItem));
 
       for (let installItem of aInstallsList)
         elements.push(createItem(installItem, true));
 
       this.showEmptyNotice(elements.length == 0);
       if (elements.length > 0) {
-        sortElements(elements, ["uiState", "name"], true);
+        let sortBy;
+        if (aType == "theme") {
+          sortBy = ["uiState", "hasPreview", "name"];
+        } else {
+          sortBy = ["uiState", "name"];
+        }
+        sortElements(elements, sortBy, true);
         for (let element of elements) {
           this._listBox.appendChild(element);
         }
       }
 
       this.filterDisabledUnsigned(showOnlyDisabledUnsigned);
       let legacyNotice = document.getElementById("legacy-extensions-notice");
       if (showLegacyInfo) {
--- a/toolkit/mozapps/extensions/content/extensions.xml
+++ b/toolkit/mozapps/extensions/content/extensions.xml
@@ -910,16 +910,17 @@
             this._description.hidden = true;
 
           // Set a previewURL for themes if one exists.
           let previewURL = this.mAddon.type == "theme" &&
             this.mAddon.screenshots &&
             this.mAddon.screenshots[0] &&
             this.mAddon.screenshots[0].url;
           this.setAttribute("previewURL", previewURL ? previewURL : "");
+          this.setAttribute("hasPreview", previewURL ? "true" : "fase");
 
           let legacyWarning = legacyExtensionsEnabled && !this.mAddon.install &&
             isLegacyExtension(this.mAddon);
           this.setAttribute("legacy", legacyWarning);
           document.getAnonymousElementByAttribute(this, "anonid", "legacy").href = SUPPORT_URL + "webextensions";
 
           if (!("applyBackgroundUpdates" in this.mAddon) ||
               (this.mAddon.applyBackgroundUpdates == AddonManager.AUTOUPDATE_DISABLE ||
--- a/toolkit/mozapps/extensions/test/browser/browser_theme_previews.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_theme_previews.js
@@ -33,21 +33,24 @@ function getThemeData(_id = id, manifest
       theme: {},
       ...manifest,
     },
     "preview.png": imageBuffer,
     ...files,
   };
 }
 
-add_task(async function testThemePreviewShown() {
+async function init(startPage) {
   gManagerWindow = await open_manager(null);
   gCategoryUtilities = new CategoryUtilities(gManagerWindow);
+  return gCategoryUtilities.openType(startPage);
+}
 
-  await gCategoryUtilities.openType("theme");
+add_task(async function testThemePreviewShown() {
+  await init("theme");
 
   await AddonTestUtils.promiseInstallXPI(getThemeData());
   let addon = await AddonManager.getAddonByID(id);
 
   ok(addon.screenshots[0].url, "The add-on has a preview URL");
   let previewURL = addon.screenshots[0].url;
 
   let doc = gManagerWindow.document;
@@ -65,8 +68,46 @@ add_task(async function testThemePreview
   await wait_for_view_load(gManagerWindow);
 
   image = doc.querySelector(".theme-screenshot");
   is(image.src, previewURL, "The previewURL is set on the detail image src");
 
   await close_manager(gManagerWindow);
   await addon.uninstall();
 });
+
+add_task(async function testThemeOrdering() {
+  // Install themes before loading the manager, if it's open they'll sort by install date.
+  let themeId = id => id + "@mochi.test";
+  let themeIds = [themeId(5), themeId(6), themeId(7), themeId(8)];
+  await AddonTestUtils.promiseInstallXPI(getThemeData(themeId(6), {name: "BBB"}));
+  await AddonTestUtils.promiseInstallXPI(getThemeData(themeId(7), {name: "CCC"}));
+  await AddonTestUtils.promiseInstallXPI(getThemeData(themeId(5), {name: "AAA"}, {previewURL: ""}));
+  await AddonTestUtils.promiseInstallXPI(getThemeData(themeId(8), {name: "DDD"}));
+
+  // Enable a theme to make sure it's first.
+  let addon = await AddonManager.getAddonByID(themeId(8));
+  addon.enable();
+
+  // Load themes now that the extensions are setup.
+  await init("theme");
+
+  // Find the order of ids for the ones we installed.
+  let list = gManagerWindow.document.getElementById("addon-list");
+  let idOrder = list.itemChildren
+    .map(row => row.getAttribute("value"))
+    .filter(id => themeIds.includes(id));
+
+  // Check the order.
+  Assert.deepEqual(
+    idOrder,
+    [
+      themeId(8), // The active theme first.
+      themeId(6), themeId(7), // With previews, ordered by name.
+      themeId(5), // The theme without a preview last.
+    ],
+    "Themes are ordered by enabled, previews, then name");
+
+  await close_manager(gManagerWindow);
+  for (let addon of await promiseAddonsByIDs(themeIds)) {
+    await addon.uninstall();
+  }
+});