From 80a211b569ee392d7ead0deff52e7b79e3bc22ef Mon Sep 17 00:00:00 2001 From: Mike Conley Date: Fri, 21 Jan 2011 15:03:10 -0500 Subject: [PATCH] Bug 624808 - Add-ons should be grouped in list view according to their status (enabled, disabled, etc). r=Mossop a=b --- .../mozapps/extensions/content/extensions.js | 95 ++++++-- .../extensions/test/browser/browser_list.js | 36 +-- .../test/browser/browser_sorting.js | 207 +++++++++++++++++- 3 files changed, 291 insertions(+), 47 deletions(-) diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js index 5ebd5689c1a1..0a896f4012ad 100644 --- a/toolkit/mozapps/extensions/content/extensions.js +++ b/toolkit/mozapps/extensions/content/extensions.js @@ -1190,8 +1190,12 @@ function createItem(aObj, aIsInstall, aIsRemote) { } function sortElements(aElements, aSortBy, aAscending) { + // aSortBy is an Array of attributes to sort by, in decending + // order of priority. + const DATE_FIELDS = ["updateDate"]; const NUMERIC_FIELDS = ["size", "relevancescore", "purchaseAmount"]; + const UISTATE_ORDER = ["enabled", "incompatible", "disabled", "blocked"] function dateCompare(a, b) { var aTime = a.getTime(); @@ -1211,47 +1215,92 @@ function sortElements(aElements, aSortBy, aAscending) { return a.localeCompare(b); } - function getValue(aObj) { + function uiStateCompare(a, b) { + // 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)); + } + + function getValue(aObj, aKey) { if (!aObj) return null; - if (aObj.hasAttribute(aSortBy)) - return aObj.getAttribute(aSortBy); + if (aObj.hasAttribute(aKey)) + return aObj.getAttribute(aKey); addon = aObj.mAddon || aObj.mInstall; if (!addon) return null; + if (aKey == "uiState") { + if (addon.isActive) + return "enabled"; + else if (!addon.isCompatible) + return "incompatible"; + else if (addon.blocklistState == Ci.nsIBlocklistService.STATE_NOT_BLOCKED) + return "disabled"; + else if (addon.isCompatible && + addon.blocklistState != Ci.nsIBlocklistService.STATE_NOT_BLOCKED) + return "blocked"; + } - return addon[aSortBy]; + + return addon[aKey]; + } + + // aSortFuncs will hold the sorting functions that we'll + // use per element, in the correct order. + var aSortFuncs = []; + + for (let i = 0; i < aSortBy.length; i++) { + var sortBy = aSortBy[i]; + + aSortFuncs[i] = stringCompare; + + if (sortBy == "uiState") + aSortFuncs[i] = uiStateCompare; + else if (DATE_FIELDS.indexOf(sortBy) != -1) + aSortFuncs[i] = dateCompare; + else if (NUMERIC_FIELDS.indexOf(sortBy) != -1) + aSortFuncs[i] = numberCompare; } - var sortFunc = stringCompare; - if (DATE_FIELDS.indexOf(aSortBy) != -1) - sortFunc = dateCompare; - else if (NUMERIC_FIELDS.indexOf(aSortBy) != -1) - sortFunc = numberCompare; aElements.sort(function(a, b) { if (!aAscending) [a, b] = [b, a]; - var aValue = getValue(a); - var bValue = getValue(b); + for (let i = 0; i < aSortFuncs.length; i++) { + var sortBy = aSortBy[i]; + var aValue = getValue(a, sortBy); + var bValue = getValue(b, sortBy); - if (!aValue && !bValue) - return 0; - if (!aValue) - return -1; - if (!bValue) - return 1; + if (!aValue && !bValue) + return 0; + if (!aValue) + return -1; + if (!bValue) + return 1; + if (aValue != bValue) { + var result = aSortFuncs[i](aValue, bValue); + + if (result != 0) + return result; + } + } + + // If we got here, then all values of a and b + // must have been equal. + return 0; - return sortFunc(aValue, bValue); }); } function sortList(aList, aSortBy, aAscending) { var elements = Array.slice(aList.childNodes, 0); - sortElements(elements, aSortBy, aAscending); + sortElements(elements, [aSortBy], aAscending); while (aList.listChild) aList.removeChild(aList.lastChild); @@ -1793,7 +1842,7 @@ var gSearchView = { function finishSearch(createdCount) { if (elements.length > 0) { - sortElements(elements, self._sorters.sortBy, self._sorters.ascending); + sortElements(elements, [self._sorters.sortBy], self._sorters.ascending); elements.forEach(function(aElement) { self._listBox.insertBefore(aElement, self._listBox.lastChild); }); @@ -2066,7 +2115,7 @@ var gListView = { self.showEmptyNotice(elements.length == 0); if (elements.length > 0) { - sortElements(elements, "name", true); + sortElements(elements, ["uiState", "name"], true); elements.forEach(function(aElement) { self._listBox.appendChild(aElement); }); @@ -2636,7 +2685,7 @@ var gUpdatesView = { self.showEmptyNotice(elements.length == 0); if (elements.length > 0) { - sortElements(elements, self._sorters.sortBy, self._sorters.ascending); + sortElements(elements, [self._sorters.sortBy], self._sorters.ascending); elements.forEach(function(aElement) { self._listBox.appendChild(aElement); }); @@ -2683,7 +2732,7 @@ var gUpdatesView = { self.showEmptyNotice(elements.length == 0); if (elements.length > 0) { self._updateSelected.hidden = false; - sortElements(elements, self._sorters.sortBy, self._sorters.ascending); + sortElements(elements, [self._sorters.sortBy], self._sorters.ascending); elements.forEach(function(aElement) { self._listBox.appendChild(aElement); }); diff --git a/toolkit/mozapps/extensions/test/browser/browser_list.js b/toolkit/mozapps/extensions/test/browser/browser_list.js index ee45d25376a9..6def2224b7ae 100644 --- a/toolkit/mozapps/extensions/test/browser/browser_list.js +++ b/toolkit/mozapps/extensions/test/browser/browser_list.js @@ -81,12 +81,12 @@ function end_test() { function get_test_items() { var tests = "@tests.mozilla.org"; - var items = []; + var items = {}; var item = gManagerWindow.document.getElementById("addon-list").firstChild; while (item) { if (item.mAddon.id.substring(item.mAddon.id.length - tests.length) == tests) - items.push(item); + items[item.mAddon.name] = item; item = item.nextSibling; } @@ -106,10 +106,10 @@ function get_class_node(parent, cls) { add_test(function() { gCategoryUtilities.openType("extension", function() { let items = get_test_items(); - is(items.length, 7, "Should be seven add-ons installed"); + is(Object.keys(items).length, 7, "Should be seven add-ons installed"); info("Addon 1"); - let addon = items[0]; + let addon = items["Test add-on"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on", "Name should be correct"); is_element_visible(get_node(addon, "version"), "Version should be visible"); @@ -147,7 +147,7 @@ add_test(function() { is(get_node(addon, "pending").textContent, "Test add-on will be disabled after you restart " + gApp + ".", "Pending message should be correct"); info("Addon 2"); - addon = items[1]; + addon = items["Test add-on 2"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 2", "Name should be correct"); is_element_visible(get_node(addon, "version"), "Version should be visible"); @@ -184,7 +184,7 @@ add_test(function() { is(get_node(addon, "pending").textContent, "Test add-on 2 will be enabled after you restart " + gApp + ".", "Pending message should be correct"); info("Addon 3"); - addon = items[2]; + addon = items["Test add-on 3"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 3", "Name should be correct"); is_element_hidden(get_node(addon, "version"), "Version should be hidden"); @@ -202,7 +202,7 @@ add_test(function() { is_element_hidden(get_node(addon, "pending"), "Pending message should be hidden"); info("Addon 4"); - addon = items[3]; + addon = items["Test add-on 4"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 4", "Name should be correct"); @@ -235,7 +235,7 @@ add_test(function() { is(get_node(addon, "pending").textContent, "Test add-on 4 will be enabled after you restart " + gApp + ".", "Pending message should be correct"); info("Addon 5"); - addon = items[4]; + addon = items["Test add-on 5"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 5", "Name should be correct"); @@ -254,7 +254,7 @@ add_test(function() { is_element_hidden(get_node(addon, "pending"), "Pending message should be hidden"); info("Addon 6"); - addon = items[5]; + addon = items["Test add-on 6"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 6", "Name should be correct"); is_element_hidden(get_class_node(addon, "disabled-postfix"), "Disabled postfix should be hidden"); @@ -286,7 +286,7 @@ add_test(function() { is_element_hidden(get_node(addon, "pending"), "Pending message should be hidden"); info("Addon 7"); - addon = items[6]; + addon = items["Test add-on 7"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 7", "Name should be correct"); @@ -343,10 +343,10 @@ add_test(function() { gCategoryUtilities.openType("plugin", function() { gCategoryUtilities.openType("extension", function() { let items = get_test_items(); - is(items.length, 7, "Should be seven add-ons installed"); + is(Object.keys(items).length, 7, "Should be seven add-ons installed"); info("Addon 1"); - let addon = items[0]; + let addon = items["Test add-on"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on", "Name should be correct"); is_element_visible(get_node(addon, "version"), "Version should be visible"); @@ -384,7 +384,7 @@ add_test(function() { is_element_hidden(get_node(addon, "pending"), "Pending message should be hidden"); info("Addon 2"); - addon = items[1]; + addon = items["Test add-on 2"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 2", "Name should be correct"); is_element_visible(get_node(addon, "version"), "Version should be visible"); @@ -421,7 +421,7 @@ add_test(function() { is_element_hidden(get_node(addon, "pending"), "Pending message should be hidden"); info("Addon 4"); - addon = items[3]; + addon = items["Test add-on 4"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 4", "Name should be correct"); @@ -454,7 +454,7 @@ add_test(function() { is_element_hidden(get_node(addon, "pending"), "Pending message should be hidden"); info("Addon 6"); - addon = items[5]; + addon = items["Test add-on 6"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 6", "Name should be correct"); is_element_visible(get_class_node(addon, "disabled-postfix"), "Disabled postfix should be visible"); @@ -486,7 +486,7 @@ add_test(function() { is_element_hidden(get_node(addon, "pending"), "Pending message should be hidden"); info("Addon 7"); - addon = items[6]; + addon = items["Test add-on 7"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on 7", "Name should be correct"); @@ -549,9 +549,9 @@ add_test(function() { }]); let items = get_test_items(); - is(items.length, 7, "Should be seven add-ons installed"); + is(Object.keys(items).length, 7, "Should be seven add-ons installed"); - let addon = items[0]; + let addon = items["Test add-on replacement"]; addon.parentNode.ensureElementIsVisible(addon); is(get_node(addon, "name").value, "Test add-on replacement", "Name should be correct"); is_element_visible(get_node(addon, "version"), "Version should be visible"); diff --git a/toolkit/mozapps/extensions/test/browser/browser_sorting.js b/toolkit/mozapps/extensions/test/browser/browser_sorting.js index 3bd9c7470b8c..64aff96c4e2f 100644 --- a/toolkit/mozapps/extensions/test/browser/browser_sorting.js +++ b/toolkit/mozapps/extensions/test/browser/browser_sorting.js @@ -13,6 +13,7 @@ function test() { gProvider = new MockProvider(); gProvider.createAddons([{ + // Enabled extensions id: "test1@tests.mozilla.org", name: "Test add-on", description: "foo", @@ -41,6 +42,121 @@ function test() { description: "foo", updateDate: new Date(2012, 12, 12, 00, 00, 00), size: 5 + }, { + // Incompatible, disabled extensions + id: "test6@tests.mozilla.org", + name: "orange Add-on", + description: "foo", + updateDate: new Date(2010, 04, 02, 00, 00, 00), + size: 142, + isCompatible: false, + isActive: false, + }, { + id: "test7@tests.mozilla.org", + name: "Blue Add-on", + description: "foo", + updateDate: new Date(2010, 04, 01, 23, 59, 59), + size: 65, + isCompatible: false, + isActive: false, + }, { + id: "test8@tests.mozilla.org", + name: "Green Add-on", + description: "foo", + updateDate: new Date(2010, 04, 03, 00, 00, 01), + size: 125, + isCompatible: false, + isActive: false, + }, { + id: "test9@tests.mozilla.org", + name: "red Add-on", + updateDate: new Date(2011, 04, 01, 00, 00, 00), + description: "foo", + isCompatible: false, + isActive: false, + }, { + id: "test10@tests.mozilla.org", + name: "Purple Add-on", + description: "foo", + updateDate: new Date(2012, 12, 12, 00, 00, 00), + size: 56, + isCompatible: false, + isActive: false, + }, { + // Disabled, compatible extensions + id: "test11@tests.mozilla.org", + name: "amber Add-on", + description: "foo", + updateDate: new Date(1978, 04, 02, 00, 00, 00), + size: 142, + isActive: false, + }, { + id: "test12@tests.mozilla.org", + name: "Salmon Add-on", + description: "foo", + updateDate: new Date(2054, 04, 01, 23, 59, 59), + size: 65, + isActive: false, + }, { + id: "test13@tests.mozilla.org", + name: "rose Add-on", + description: "foo", + updateDate: new Date(2010, 04, 02, 00, 00, 01), + size: 125, + isActive: false, + }, { + id: "test14@tests.mozilla.org", + name: "Violet Add-on", + updateDate: new Date(2010, 05, 01, 00, 00, 00), + description: "foo", + isActive: false, + }, { + id: "test15@tests.mozilla.org", + name: "white Add-on", + description: "foo", + updateDate: new Date(2010, 04, 12, 00, 00, 00), + size: 56, + isActive: false, + }, { + // Blocked extensions + id: "test16@tests.mozilla.org", + name: "grimsby Add-on", + description: "foo", + updateDate: new Date(2010, 04, 01, 00, 00, 00), + size: 142, + isActive: false, + blocklistState: Ci.nsIBlocklistService.STATE_SOFTBLOCKED, + }, { + id: "test17@tests.mozilla.org", + name: "beamsville Add-on", + description: "foo", + updateDate: new Date(2010, 04, 8, 23, 59, 59), + size: 65, + isActive: false, + blocklistState: Ci.nsIBlocklistService.STATE_SOFTBLOCKED, + }, { + id: "test18@tests.mozilla.org", + name: "smithville Add-on", + description: "foo", + updateDate: new Date(2010, 04, 03, 00, 00, 01), + size: 125, + isActive: false, + blocklistState: Ci.nsIBlocklistService.STATE_OUTDATED, + }, { + id: "test19@tests.mozilla.org", + name: "dunnville Add-on", + updateDate: new Date(2010, 04, 02, 00, 00, 00), + description: "foo", + isActive: false, + blocklistState: Ci.nsIBlocklistService.STATE_SOFTBLOCKED, + }, { + id: "test20@tests.mozilla.org", + name: "silverdale Add-on", + description: "foo", + updateDate: new Date(2010, 04, 12, 00, 00, 00), + size: 56, + isActive: false, + blocklistState: Ci.nsIBlocklistService.STATE_BLOCKED, }]); open_manager("addons://list/extension", function(aWindow) { @@ -63,7 +179,7 @@ function set_order(aSortBy, aAscending) { elements.push(node); node = node.nextSibling; } - gManagerWindow.sortElements(elements, aSortBy, aAscending); + gManagerWindow.sortElements(elements, ["uiState", aSortBy], aAscending); elements.forEach(function(aElement) { list.appendChild(aElement); }); @@ -85,12 +201,28 @@ function check_order(aExpectedOrder) { // Tests that ascending name ordering was the default add_test(function() { + check_order([ "test2@tests.mozilla.org", "test4@tests.mozilla.org", "test3@tests.mozilla.org", "test5@tests.mozilla.org", - "test1@tests.mozilla.org" + "test1@tests.mozilla.org", + "test7@tests.mozilla.org", + "test8@tests.mozilla.org", + "test6@tests.mozilla.org", + "test10@tests.mozilla.org", + "test9@tests.mozilla.org", + "test11@tests.mozilla.org", + "test13@tests.mozilla.org", + "test12@tests.mozilla.org", + "test14@tests.mozilla.org", + "test15@tests.mozilla.org", + "test17@tests.mozilla.org", + "test19@tests.mozilla.org", + "test16@tests.mozilla.org", + "test20@tests.mozilla.org", + "test18@tests.mozilla.org", ]); run_next_test(); @@ -100,12 +232,30 @@ add_test(function() { add_test(function() { set_order("updateDate", false); + // When we're ascending with updateDate, it's from newest + // to oldest. + check_order([ "test5@tests.mozilla.org", "test3@tests.mozilla.org", "test1@tests.mozilla.org", "test2@tests.mozilla.org", - "test4@tests.mozilla.org" + "test4@tests.mozilla.org", + "test10@tests.mozilla.org", + "test9@tests.mozilla.org", + "test8@tests.mozilla.org", + "test6@tests.mozilla.org", + "test7@tests.mozilla.org", + "test12@tests.mozilla.org", + "test14@tests.mozilla.org", + "test15@tests.mozilla.org", + "test13@tests.mozilla.org", + "test11@tests.mozilla.org", + "test20@tests.mozilla.org", + "test17@tests.mozilla.org", + "test18@tests.mozilla.org", + "test19@tests.mozilla.org", + "test16@tests.mozilla.org", ]); set_order("updateDate", true); @@ -115,7 +265,22 @@ add_test(function() { "test2@tests.mozilla.org", "test1@tests.mozilla.org", "test3@tests.mozilla.org", - "test5@tests.mozilla.org" + "test5@tests.mozilla.org", + "test7@tests.mozilla.org", + "test6@tests.mozilla.org", + "test8@tests.mozilla.org", + "test9@tests.mozilla.org", + "test10@tests.mozilla.org", + "test11@tests.mozilla.org", + "test13@tests.mozilla.org", + "test15@tests.mozilla.org", + "test14@tests.mozilla.org", + "test12@tests.mozilla.org", + "test16@tests.mozilla.org", + "test19@tests.mozilla.org", + "test18@tests.mozilla.org", + "test17@tests.mozilla.org", + "test20@tests.mozilla.org", ]); run_next_test(); @@ -130,7 +295,22 @@ add_test(function() { "test4@tests.mozilla.org", "test3@tests.mozilla.org", "test5@tests.mozilla.org", - "test1@tests.mozilla.org" + "test1@tests.mozilla.org", + "test7@tests.mozilla.org", + "test8@tests.mozilla.org", + "test6@tests.mozilla.org", + "test10@tests.mozilla.org", + "test9@tests.mozilla.org", + "test11@tests.mozilla.org", + "test13@tests.mozilla.org", + "test12@tests.mozilla.org", + "test14@tests.mozilla.org", + "test15@tests.mozilla.org", + "test17@tests.mozilla.org", + "test19@tests.mozilla.org", + "test16@tests.mozilla.org", + "test20@tests.mozilla.org", + "test18@tests.mozilla.org", ]); set_order("name", false); @@ -140,7 +320,22 @@ add_test(function() { "test5@tests.mozilla.org", "test3@tests.mozilla.org", "test4@tests.mozilla.org", - "test2@tests.mozilla.org" + "test2@tests.mozilla.org", + "test9@tests.mozilla.org", + "test10@tests.mozilla.org", + "test6@tests.mozilla.org", + "test8@tests.mozilla.org", + "test7@tests.mozilla.org", + "test15@tests.mozilla.org", + "test14@tests.mozilla.org", + "test12@tests.mozilla.org", + "test13@tests.mozilla.org", + "test11@tests.mozilla.org", + "test18@tests.mozilla.org", + "test20@tests.mozilla.org", + "test16@tests.mozilla.org", + "test19@tests.mozilla.org", + "test17@tests.mozilla.org", ]); run_next_test();