Closed Bug 553967 Opened 12 years ago Closed 12 years ago

Add a direct way to get to the AddonInstall from an Addon that is pending upgrade

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.3a5

People

(Reporter: mossop, Assigned: mossop)

References

Details

(Whiteboard: [rewrite])

Attachments

(1 file, 1 obsolete file)

No description provided.
I think the most consistent way to do this would be to have an "install" property on Addons that are pending install. So Addon.install points to the AddonInstall and AddonInstall.addon points to the new Addon and Addon.pendingUpgrade.install would point to the AddonInstall upgrading an Addon.
http://hg.mozilla.org/projects/addonsmgr/rev/24ea43f4df86
Flags: in-testsuite+
Whiteboard: [rewrite] → [rewrite][fixed-in-addonsmgr]
I assume we do not need a manual test here. If I'm wrong please re-flag.
Assignee: dtownsend → bmcbride
Status: NEW → ASSIGNED
Flags: in-litmus-
Oops, I missed that. Correct - automated testing covers this completely.
This doesn't seem to have been implemented in the way described in comment 1, looks like you've put the install property on the Addon that is pending upgrade as opposed to the Addon that the AddonInstall is installing.
Backed out the original changeset with http://hg.mozilla.org/projects/addonsmgr/rev/3399677bafaa and landed a fixed version at http://hg.mozilla.org/projects/addonsmgr/rev/8aec62049ba2
Whiteboard: [rewrite][fixed-in-addonsmgr] → [rewrite][fixed-in-addonsmgr][needs-review]
Attached patch Add Addon.install property (obsolete) — Splinter Review
Adds an Addon.install property to get the AddonInstall that is installing it.
Attachment #435771 - Flags: review?(robert.bugzilla)
Assignee: bmcbride → dtownsend
Comment on attachment 435771 [details] [diff] [review]
Add Addon.install property

Looks good

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>...
>@@ -3820,6 +3826,14 @@ function AddonWrapper(addon) {
>     addon.updateAutomatically = val;
>   });
> 
>+  this.__defineGetter__("install", function() {
>+    if (!("_install" in addon))
>+      return null;
>+    if (!addon._install)
>+      return null;
Why not check both on one line?
Attachment #435771 - Flags: review?(robert.bugzilla) → review+
Whiteboard: [rewrite][fixed-in-addonsmgr][needs-review] → [rewrite][fixed-in-addonsmgr]
Attached patch patch rev 2Splinter Review
Ready to land
Attachment #435771 - Attachment is obsolete: true
Attachment #437077 - Flags: review+
Whiteboard: [rewrite][fixed-in-addonsmgr] → [rewrite][fixed-in-addonsmgr][needs-landing]
http://hg.mozilla.org/mozilla-central/rev/0e4fdfeee54c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [rewrite][fixed-in-addonsmgr][needs-landing] → [rewrite]
Target Milestone: --- → mozilla1.9.3a5
Marking as 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.