Bug 733582: Post 1.4 refactoring fixes. #4

Merged
merged 2 commits into from Sep 10, 2012

Conversation

Projects
None yet
2 participants
Member

ochameau commented Mar 22, 2012

Here is a first revision that address review comments made in PR #3.

@Gozala Gozala commented on an outdated diff Mar 22, 2012

lib/addons-builder-helper.js
},
uninstall: function uninstall(callback) {
- if (currExtension) {
- var oldExtension = currExtension;
- currExtension = null;
- oldExtension.unload(callback);
+ if (!currentAddonId) {
+ if (typeof callback == "function")
+ callback();
+ }
+ else {
+ let addonId = currentAddonId;
+ currentAddonId = null;
+ AddonInstall.uninstall(addonId, callback);
@Gozala

Gozala Mar 22, 2012

Member

I wonder if callback arguments will be the same in both cases.

@Gozala Gozala commented on an outdated diff Mar 22, 2012

lib/addon-install.js
- get isInstalled() "_addon" in this,
+ // Order AddonManager to install the addon
+ AddonManager.getInstallForFile(file, function(install) {
+ install.addListener(listener);
+ install.install();
@Gozala

Gozala Mar 22, 2012

Member

too many install names man. maybe you should call argument installer ?

installer.addListener(listener);
installer.install();
Member

Gozala commented Mar 22, 2012

Just a nits otherwise looks good to me!

Gozala was assigned Aug 29, 2012

Member

ochameau commented Aug 29, 2012

@Gozala can you give it another look as I moved addon-install to SDK and you reviewed that patch when addon-install was still hosted in the builder.

@ochameau ochameau added a commit that referenced this pull request Sep 10, 2012

@ochameau ochameau Merge pull request #4 from ochameau/bug/733582-post-1.4-refactoring-f…
…ixes

Bug 733582: Post 1.4 refactoring fixes. r=@erikvold
0c040e6

@ochameau ochameau merged commit 0c040e6 into mozilla:master Sep 10, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment