Bug 1429488 - Part 8: Fix mobile about:addons code for enabling/disabling themes. r=aswan

about:addons on Android has some logic to ensure that only one theme can be
active at the same time.

At some point however, themes must have learned to take care of themselves in
that regard, which means that this code
a) has become unnecessary, and
b) with Webextension themes it can actually cause a harmful race condition:

Explicitly disabling the previously active theme implicitly enables the default
theme, so what happens is that depending on the size of the Webextension theme
to be enabled, the new theme can be enabled before the previous disable() call
completes. This leads to the new theme then immediately being disabled again as
the previous disable() call finally gets around to enabling the default theme.
The smaller the Webextension theme to be enabled and the faster it loads, the
more likely this is to happen.

We still need to manually fix up the disabled state of themes in the UI, though,
so that it shows the correct state without a reload. Therefore, we take the
opportunity to fix the problem that up until now, disabling a theme wouldn't
mark the default theme as enabled in the UI, unless you manually reloaded the
page afterwards.

Differential Revision: https://phabricator.services.mozilla.com/D10732

--HG--
extra : source : 995ee600dc4eacac1f2d6c2409d83227c45f9d35
extra : histedit_source : 935f0d5c95bfc02acd296003fb65790cd9aa9a9d
This commit is contained in:
Jan Henning 2018-11-01 22:08:59 +01:00
parent 33c4271292
commit 066dfd8ff4

View File

@ -521,20 +521,33 @@ var Addons = {
return addon.enable(); return addon.enable();
} }
function updateOtherThemeStateInUI(item) {
if (aValue) {
// Mark the previously enabled theme as disabled.
if (item.addon.isActive) {
item.setAttribute("isDisabled", true);
return true;
}
// The current theme is being disabled - enable the default theme.
} else if (item.addon.id == "default-theme@mozilla.org") {
item.removeAttribute("isDisabled");
return true;
}
return false;
}
let opType; let opType;
if (addon.type == "theme") { if (addon.type == "theme") {
if (aValue) { // Themes take care of themselves to make sure only one is active at the
// We can have only one theme enabled, so disable the current one if any // same time, but we need to fix up the state of other themes in the UI.
let list = document.getElementById("addons-list"); let list = document.getElementById("addons-list");
let item = list.firstElementChild; let item = list.firstElementChild;
while (item) { while (item) {
if (item.addon && (item.addon.type == "theme") && (item.addon.isActive)) { if (item.addon && (item.addon.type == "theme") &&
item.addon.disable(); updateOtherThemeStateInUI(item)) {
item.setAttribute("isDisabled", true); break;
break;
}
item = item.nextSibling;
} }
item = item.nextSibling;
} }
setDisabled(addon, !aValue); setDisabled(addon, !aValue);
} else if (addon.type == "locale") { } else if (addon.type == "locale") {