Update checks for lightweight themes

VERIFIED FIXED in mozilla1.9.3a5

Status

()

Toolkit
Add-ons Manager
P1
normal
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

Trunk
mozilla1.9.3a5
Points:
---
Bug Flags:
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rewrite])

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

8 years ago
http://hg.mozilla.org/projects/addonsmgr/rev/adfa6cfa9d81
Flags: in-testsuite+
Flags: in-litmus-
Whiteboard: [rewrite] → [rewrite][fixed-in-addonsmgr][needs-review]
(Assignee)

Comment 2

8 years ago
Created attachment 435765 [details] [diff] [review]
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.
Attachment #435765 - Flags: review?(robert.bugzilla)
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);
>       }
>+
Attachment #435765 - Flags: review?(robert.bugzilla) → review+
Whiteboard: [rewrite][fixed-in-addonsmgr][needs-review] → [rewrite][fixed-in-addonsmgr]
(Assignee)

Comment 4

8 years ago
Created attachment 437333 [details] [diff] [review]
patch rev 2

Comments addressed
Attachment #435765 - Attachment is obsolete: true
Attachment #437333 - Flags: review+
(Assignee)

Updated

8 years ago
Whiteboard: [rewrite][fixed-in-addonsmgr] → [rewrite][fixed-in-addonsmgr][needs-landing]
(Assignee)

Comment 5

7 years ago
http://hg.mozilla.org/mozilla-central/rev/5fea6417c8f2
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Whiteboard: [rewrite][fixed-in-addonsmgr][needs-landing] → [rewrite]
Target Milestone: --- → mozilla1.9.3a5
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.
(Assignee)

Comment 7

7 years ago
(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.
Verified fixed based on check-in and passing automated tests.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.