Skip to content

Commit

Permalink
Bug 1738325 - Part 2 - Show expired but retained themes among the reg…
Browse files Browse the repository at this point in the history
…ular themes in about:addons. r=rpl,dao

Differential Revision: https://phabricator.services.mozilla.com/D130940
  • Loading branch information
htwyford committed Nov 17, 2021
1 parent dbd64d9 commit 1ef1076
Show file tree
Hide file tree
Showing 5 changed files with 136 additions and 18 deletions.
26 changes: 26 additions & 0 deletions browser/themes/BuiltInThemes.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,32 @@ class _BuiltInThemes {
await Promise.all(installPromises);
}

/**
* @param {string} id
* A theme's ID.
* @returns {boolean}
* Returns true if the theme is expired. False otherwise.
* @note This looks up the id in a Map rather than accessing a property on
* the addon itself. That makes calls to this function O(m) where m is the
* total number of built-in themes offered now or in the past. Since we
* are using a Map, calls are O(1) in the average case.
*/
themeIsExpired(id) {
let themeInfo = this.builtInThemeMap.get(id);
return themeInfo?.expiry && new Date(themeInfo.expiry) < new Date();
}

/**
* @param {string} id
* The theme's id.
* @return {boolean}
* True if the theme with id `id` is both expired and retained. That is,
* the user has the ability to use it after its expiry date.
*/
isRetainedExpiredTheme(id) {
return retainedThemes.includes(id) && this.themeIsExpired(id);
}

/**
* Uninstalls themes after they expire. If the expired theme is active, then
* it is not uninstalled. Instead, it is saved so that the user can use it
Expand Down
19 changes: 15 additions & 4 deletions toolkit/mozapps/extensions/content/aboutaddons.js
Original file line number Diff line number Diff line change
Expand Up @@ -4596,7 +4596,7 @@ gViewController.defineView("list", async type => {

// If monochromatic themes are enabled and any are builtin to Firefox, we
// display those themes together in a separate subsection.
let isMonochromaticTheme = addon =>
const isMonochromaticTheme = addon =>
addon.id.endsWith("[email protected]");

let frag = document.createDocumentFragment();
Expand All @@ -4615,21 +4615,32 @@ gViewController.defineView("list", async type => {
!addon.hidden &&
!addon.isActive &&
!isPending(addon, "uninstall") &&
!isMonochromaticTheme(addon),
// For performance related details about this check see the
// documentation for themeIsExpired in BuiltInThemeConfig.jsm.
(!isMonochromaticTheme(addon) ||
BuiltInThemes.isRetainedExpiredTheme(addon.id)),
},
];
list.setSections(sections);
frag.appendChild(list);

if (type == "theme") {
const areColorwayThemesInstalled = async () =>
(await AddonManager.getAllAddons()).some(
addon =>
isMonochromaticTheme(addon) && !BuiltInThemes.themeIsExpired(addon.id)
);
if (type == "theme" && (await areColorwayThemesInstalled())) {
let monochromaticList = document.createElement("addon-list");
monochromaticList.classList.add("monochromatic-addon-list");
monochromaticList.type = type;
monochromaticList.setSections([
{
headingId: type + "-monochromatic-heading",
subheadingId: type + "-monochromatic-subheading",
filterFn: addon => !addon.hidden && isMonochromaticTheme(addon),
filterFn: addon =>
!addon.hidden &&
isMonochromaticTheme(addon) &&
!BuiltInThemes.themeIsExpired(addon.id),
},
]);
frag.appendChild(monochromaticList);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@ const SUPPORT_URL = Services.urlFormatter.formatURL(
);
const REMOVE_SUMO_URL = SUPPORT_URL + "cant-remove-addon";

const SECTION_INDEXES = {
enabled: 0,
disabled: 1,
};
function getSection(doc, type) {
return doc.querySelector(`section[section="${SECTION_INDEXES[type]}"]`);
}

function getTestCards(root) {
return root.querySelectorAll('addon-card[addon-id$="@mochi.test"]');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@
const { AddonTestUtils } = ChromeUtils.import(
"resource://testing-common/AddonTestUtils.jsm"
);
const { BuiltInThemes } = ChromeUtils.import(
"resource:///modules/BuiltInThemes.jsm"
);

AddonTestUtils.initMochitest(this);

const kTestThemeId = "[email protected]";

add_task(async function testMonochromaticList() {
// Install test theme before loading view.
const themeXpi = AddonTestUtils.createTempWebExtensionFile({
manifest: {
name: "Monochromatic Theme",
applications: { gecko: { id: "[email protected]" } },
applications: { gecko: { id: kTestThemeId } },
theme: {},
},
});
Expand Down Expand Up @@ -49,7 +54,8 @@ add_task(async function testMonochromaticList() {
"Subheader string is correct."
);

// Check that the test theme is in the colorways section.
// Check that the test theme is in the colorways section. It should be there
// because it hasn't yet expired.
let card = colorwayList.querySelector(
"addon-card[addon-id='[email protected]']"
);
Expand All @@ -61,19 +67,18 @@ add_task(async function testMonochromaticList() {

// Check that the test theme is in the enabled section.
let addon = await AddonManager.getAddonByID("[email protected]");
let enabledSection = doc.querySelector("section[section='0']");
let enabledSection = getSection(doc, "enabled");
let mutationPromise = BrowserTestUtils.waitForMutationCondition(
enabledSection,
{ childList: true },
() =>
enabledSection.children.length > 1 &&
enabledSection.children[1].getAttribute("addon-id") ==
"[email protected]"
enabledSection.children[1].getAttribute("addon-id") == kTestThemeId
);
await addon.enable();
await mutationPromise;
let enabledCard = enabledSection.querySelector(
"addon-card[addon-id='[email protected]']"
`addon-card[addon-id='${kTestThemeId}']`
);
ok(
enabledSection.contains(enabledCard),
Expand Down Expand Up @@ -104,3 +109,78 @@ add_task(async function testMonochromaticList() {
await closeView(win);
await themeAddon.uninstall(true);
});

add_task(async function testExpiredThemes() {
const themeXpi = AddonTestUtils.createTempWebExtensionFile({
manifest: {
name: "Monochromatic Theme",
applications: { gecko: { id: kTestThemeId } },
theme: {},
},
});

// Make the test theme appear expired.
let yesterday = new Date();
yesterday.setDate(yesterday.getDate() - 1);
yesterday = yesterday.toISOString().split("T")[0];
// Add the test theme to our list of built-in themes so that aboutaddons.js
// will think this theme is expired.
BuiltInThemes.builtInThemeMap.set(kTestThemeId, {
version: "1.0",
expiry: yesterday,
// We use the manifest from Light theme since we know it will be in-tree
// indefinitely.
path: "resource://builtin-themes/light/",
});
registerCleanupFunction(() => {
BuiltInThemes.builtInThemeMap.delete(kTestThemeId);
});

// Make the test theme appear retained.
const retainedThemePrefName = "browser.theme.retainedExpiredThemes";
Services.prefs.setStringPref(
retainedThemePrefName,
JSON.stringify([kTestThemeId])
);
registerCleanupFunction(() => {
Services.prefs.clearUserPref(retainedThemePrefName);
});

const expiredAddon = await AddonManager.installTemporaryAddon(themeXpi);
let addon = await AddonManager.getAddonByID(kTestThemeId);
await addon.disable();

let win = await loadInitialView("theme");
let doc = win.document;

let colorwayList = doc.querySelector(".monochromatic-addon-list");

// We need branching logic here since the outcome depends on whether there
// are active non-test Colorway themes when the test runs.
if (colorwayList) {
let card = colorwayList.querySelector(
`addon-card[addon-id='${kTestThemeId}']`
);
ok(
!colorwayList.contains(card),
"Colorways section does not contain expired theme."
);
} else {
ok(
true,
"The Colorways section is not in the DOM because all Colorway themes are expired."
);
}

let disabledSection = getSection(doc, "disabled");
let card = disabledSection.querySelector(
`addon-card[addon-id='${kTestThemeId}']`
);
ok(
disabledSection.contains(card),
"The regular, non-Colorways 'Disabled' section contains the expired theme."
);

await closeView(win);
await expiredAddon.uninstall(true);
});
9 changes: 9 additions & 0 deletions toolkit/mozapps/extensions/test/browser/head.js
Original file line number Diff line number Diff line change
Expand Up @@ -1594,6 +1594,15 @@ async function loadInitialView(type, opts) {
return win;
}

function getSection(doc, type) {
const SECTION_INDEXES = {
enabled: 0,
disabled: 1,
colorway: 2,
};
return doc.querySelector(`section[section="${SECTION_INDEXES[type]}"]`);
}

function waitForViewLoad(win) {
return wait_for_view_load(win, undefined, true);
}
Expand Down

0 comments on commit 1ef1076

Please sign in to comment.