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 508703 fa0a79862921e6c99b9bb8e372f593dc718d258c
parent 508702 e743bb11887dd30522a13ca6b04b1cd0040beedd
child 508704 4ee2c005d4ff7ae67b50f4fe96d6e336d55c2198
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [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();
+  }
+});