565340 - [ACR Extension] Uninstall action fails in new Add-ons Manager

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Compatibility Tools, defect, P1)

Description

[Brian King [:kinger]]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a5pre) Gecko/20100511 Minefield/3.7a5pre (.NET CLR 3.5.30729)

In a build with the new add-ons manager, I get this error on startup.

Error: Components.classes['@mozilla.org/extensions/manager;1'] is undefined
Source File: chrome://acr/content/model/acr.js
Line: 226

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

New addons manager uses a completely different API - that XPCOM component is no more. Instead, there's a JavaScript Module (JSM) to import:
resource://gre/modules/AddonManager.jsm

For the new API, see documentation at:
https://wiki.mozilla.org/Extension_Manager:API_Rewrite
And:
https://wiki.mozilla.org/Extension_Manager:API_Rewrite:API

Comment 2

[David McNamara [:mackers]]

Fixed in r67270.

Comment 3

[Brian King [:kinger]]

Dave, all the prefs are set to true on uninstall. Shouldn't they be reset/removed?

Also, if I Undo, the prefs are not reverted.

Comment 4

[David McNamara [:mackers]]

The prefs are set to whatever they were when the add-on was first installed.

Comment 5

[Brian King [:kinger]]

(In reply to comment #4)
> The prefs are set to whatever they were when the add-on was first installed.

I looked at the code and if the pref does not exist at startup then the previous is set to true. I think it should be smarter so that on lastrun it does not set it to true but removes/resets it.

Also it sets one value for all prefs, instead of detecting each one individually. Not sure if that is needed because it is an edge case for users to manually change the per version prefs. Thoughts?

Comment 6

[David McNamara [:mackers]]

(In reply to comment #5)
> Also it sets one value for all prefs, instead of detecting each one
> individually. Not sure if that is needed because it is an edge case for users
> to manually change the per version prefs. Thoughts?

It would be more polite to preserve the original status of each compatibility pref, even if it is an edge case. Technically, it is not too difficult to achieve.

Comment 7

[Brian King [:kinger]]

The only gotcha is that the user could change individual prefs between install and uninstall time. But we can have a prefs listener to detect that. Not sure if we need that though because for each it is either A or B and reverting back on uninstall to install state will not overwrite users intentions.

Comment 8

[David McNamara [:mackers]]

New behaviour is:

- on uninstall, add-on reverts each check compatibility pref back to what its state was when the add-on was installed. If the pref wasn't present during install, then it will be cleared (removed) during uninstall.

- each check compatibility pref state is preserved and reverted individually.

r69024

Comment 9

[David McNamara [:mackers]]

Re: undo uninstall. This has been fixed for the new EM and the legacy EM. Undoing an uninstall action resets the add-on prefs to the correct state.

r69027

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can we get a new dev version of the add-on for testing?

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with the latest dev build of ACR with Minefield and Firefox 3.6.6.