553113 - Update checks for lightweight themes

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Comment 1

[Dave Townsend [:mossop]]

http://hg.mozilla.org/projects/addonsmgr/rev/adfa6cfa9d81

Comment 2

[Dave Townsend [:mossop]]

Attachment: Perform update check for the current lightweight theme

Adds update checking for the current lightweight theme to match what happens on trunk. May want to switch this to using the new API properly in the future.

Comment 3

[Robert Strong (they/them - no direct email)]

Comment on attachment 435765 [details] [diff] [review]
Perform update check for the current lightweight theme

>diff --git a/toolkit/mozapps/extensions/LightweightThemeManager.jsm b/toolkit/mozapps/extensions/LightweightThemeManager.jsm
>--- a/toolkit/mozapps/extensions/LightweightThemeManager.jsm
>+++ b/toolkit/mozapps/extensions/LightweightThemeManager.jsm
>@@ -129,26 +129,26 @@ var LightweightThemeManager = {
>                                      JSON.stringify(aData));
> 
>     if (aData) {
>-      let theme = this.getUsedTheme(aData.id);
>-      // TODO detect if it is an install and act accordingly
>-      if (!theme) {
>+      var theme = this.getUsedTheme(aData.id);
>+      var isInstall = !theme || theme.version != aData.version;
Why the switch from let to var? Looks like both of these could be let

>+      if (isInstall) {
>+        var oldWrapper = theme ? new AddonWrapper(theme) : null;
>         var wrapper = new AddonWrapper(aData);
>         AddonManagerPrivate.callInstallListeners("onExternalInstall", null,
>-                                                 wrapper, null, false);
>+                                                 wrapper, oldWrapper, false);
>         AddonManagerPrivate.callAddonListeners("onInstalling", wrapper, false);
>       }
>+

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Comments addressed

Comment 5

[Dave Townsend [:mossop]]

http://hg.mozilla.org/mozilla-central/rev/5fea6417c8f2

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Does it mean updates can not be checked manually via the UI or is the context menu entry just disabled for now? If it's the latter case I would file a new bug.

Comment 7

[Dave Townsend [:mossop]]

(In reply to comment #6)
> Does it mean updates can not be checked manually via the UI or is the context
> menu entry just disabled for now? If it's the latter case I would file a new
> bug.

We're not going to offer users the ability to check for updates to personas.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Verified fixed based on check-in and passing automated tests.