340339 - Need reliable uninstall/disable hook for extensions

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Extensions can use various ugly first-startup hacks to install registry entries or configure things appropriately, but there's really no decent way for them to detect that they've been uninstalled or disabled and undo the corresponding changes.  This sucks for a number of things, including support for additional content types and protocols.

A simple JS hook called on disable/uninstall (maybe even before upgrade?), and indicated via an entry in install.rdf, would represent a huge improvement from the perspective of extension authors who want to provide good uninstall behaviour.

I propose that the hook run immediately upon disable/uninstall being done by the user, rather than waiting until the next startup.  Extensions that are being disabled due to incompatible app update would need to have their disable hook loaded and run on that startup, but would just need to be written to cope with their custom components not being available, etc.  Let the perfect be not the enemy of the much better, here.

Doing this for disable, which I think is very much desired, probably requires a symmetric hook for re-enable.

Comment 1

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

It isn't any more decent than the way extensions have to detect first run but there is already the following which several extensions have been using
http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/extensions/public/nsIExtensionManager.idl#620

Comment 2

[Simon Bünzli]

The obvious question being: what should happen for extensions disabled/uninstalled in safe mode? See also bug 258301.

Comment 3

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

and the not as obvious: extensions can be uninstalled by simply removing the extension's folder.

Still though, a solution can be created that works in most circumstances which is what I believe shaver is looking for.

Comment 4

[Dave Townsend [:mossop]]

Handling it for the general case (uninstall from a standard running app) should be pretty simple. Handling the case of removing a folder is I suspect pretty much impossible. I think that if you are uninstalling an extension in safe mode then maybe you don't want it to be doing anything else since it's potentially already doing bad things?

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Not a blocker, though it sounds like something we should do. Is FUEL addressing anything like this?

Comment 6

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Don't think it's on the plan for FUEL, though it could probably be added as something like

myExt.addEventListener("uninstall", function (){ ... } );

I'd take a patch there as willingly as to the EM core. :)

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Landed in FUEL 0.1:

myExt.events.addListener("uninstall", function() { ... } );

We wrap the "em-action-requested" notification from the EM. Currently, we only support "uninstall", but should add "disabled" and "enabled"

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Show-off.

Comment 9

[Dave Townsend [:mossop]]

I think from our conversation yesterday this already basically exists, we just need some docs on how to use it properly.

Comment 10

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Wait, really? The "em-action-requested" notification is great, but I thought it only tells you that your extension has been marked for uninstall...

Comment 11

[Dave Townsend [:mossop]]

Yes, but all you really need to do is track that and then on shutdown (through a quit-application notification) do whatever you need.

I believe mfinkle is also thinking about being able to get the status of the extension through fuel so again on shutdown you could just check if your extension is in a state to be shutdown.

Comment 12

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

One problem (probably more) with this is that an extension that is disabled will never get the uninstall notification. A much more complex solution will be needed. :(

Comment 13

[Dave Townsend [:mossop]]

Well I think extensions could just listen for disable and uninstall in the same way.

I certainly don't think we should be somehow allowing add-ons that have been disabled to run and do something. They've been disabled for a reason.

Comment 14

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

Point understood though one of the reasons may be that the user didn't like the add-on and then later decided to uninstall the add-on. The add-on may then have a cleanup routine that wouldn't run since it is disabled. The point I am trying to make is that this is only a partial solution and for all practical purposes the uninstall / disable / enable / etc. hooks have existed for quite some time if the goal is to provide functionality as you have stated with these caveats.

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #14)
> The point I am trying
> to make is that this is only a partial solution and for all practical purposes
> the uninstall / disable / enable / etc. hooks have existed for quite some time
> if the goal is to provide functionality as you have stated with these caveats.
> 

That's true, but I have a suspicion that caveats of some kind will always exist. I believe that we need a best practice for why an extension would use the uninstall/disable/enable hooks. Completely removing preferences may not be the compelling reason (fe: Firefox doesn't remove a profile when uninstalling).

So if we can document _why_ an extension would use the hooks, we can better determine if the current caveats are showstoppers or not.

Comment 16

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

Sounds good... my concern is that the only change made by this bug is we now send an event along with an observer notification instead of just an observer notification so the status quo hasn't really changed much and I don't think that is all that was asked for in this bug report.

Comment 17

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

As recent reports have been shown, this issue came up a couple of times recently in cohesion with the Add-on Compatibility Reporter. A lot of people seem to run Firefox in Safe Mode when something goes bad. Then they decided to remove the ACR, with the impression that the compatibility prefs are getting reverted, and incompatible add-ons are deactivated. But that's not the case, because removing the ACR in safe mode doesn't run its uninstall hook. As result all the compatibility preferences stay active.

Would there be a way to solve problems like those? Could those uninstall hooks be handled separately to the extension code?

Comment 18

[Dave Townsend [:mossop]]

Extensions have no way to run code in safe mode currently, I'm not sure we'd want to add one either since that really violates the point of safe mode.

Comment 19

[Dave Townsend [:mossop]]

Restartless add-ons have this now, I'm not interested in providing anything to non-restartless add-ons that they don't already have.