Include a separate uninstall for plugins

[arickmann]

There is a clear difference between deactivate and uninstall. Deactivating a plugin should not remove all of its settings from the database but currently there is no central way for plugins to uninstall, and therefore no reminder to plugin developers to do so.

The plugins.php file can accomodate a separate uninstaller if one is included with the plugin.

I have attached a plugin that shows how it can be achieved.

[arickmann]

Plugin that adds uninstallation capability.

[DD32]

In my mind it shouldnt be a seperate file which is checked for, Some plugins use a folder, Some are simply a single file plonked into the plugins folder.

I think it should probably be something similar to this:

* Click uninstall
* Page loads and includes active plugins, if specified plugin is not activated, include it anyway
* plugin uses register_uninstall_hook() to register an uninstall proceedure for itself
* If hook has been registered for current plugin:
   * plugins.php calls 'uninstall_plugin-{$plugin_name}'
   * Redirect to main page "Plugin uninstalled"(Possibly at this stage the files could be deleted if requested, That fits in with the auto-install of plugins in another ticket)
  Else:
   * Redirect to main page "Plugin has not registered an uninstall method"

I'd suggest something similar to Gallery 2's plugin install/uninstall method, I was pretty impressed by that.

[darkdragon]

I agree with DD32, it should be a hook.

[arickmann]

I agree that it should be a hook, but I was wary about including a plugin file that is not active; there could be a number of reasons why it isn't active.

If it can be included in a way that activates the hook, doesn't break any class references, but doesn't run any other code then that would be ideal.

[DD32]

Replying to arickmann:

I agree that it should be a hook, but I was wary about including a plugin file that is not active; there could be a number of reasons why it isn't active.

Thats true, I guess if WP was to only load what it needs to (ie. no other plugins, and limited functions) when uninstalling a deactivated plugin it might work.. But if the user wants to uninstall the plugin because its corrupt, or malicious, then idealy the user will not want WP to load the plugin.

[arickmann]

Replying to DD32:

Thats true, I guess if WP was to only load what it needs to (ie. no other plugins, and limited functions) when uninstalling a deactivated plugin it might work.. But if the user wants to uninstall the plugin because its corrupt, or malicious, then idealy the user will not want WP to load the plugin.

Ok. So the way I see it there are two problems here:

1. How do we ring-fence the undesirable code;
   
2. How do we choose to run the uninstall code;
   

This is what I now have in mind. If the solution seems reasonable I will consider the actual code necessary:

• The plugin calls register_uninstaller( $plugin_path ); when it is activated which adds it to a list of plugins with uninstallers;
  

• The developer wraps the active plugin content in an if statement that evaluates whether the plugin is active.
  

• If the plugin is not active, and has an uninstaller, the option is available to uninstall it and if called it runs wrapped in an output buffer in the same way as when it is initially installed.
  

• If successful it removes itself from the list of uninstallers.

[arickmann]

Version 0.2 that uses a hook.

[arickmann]

A sample plugin that implements the uninstall process.

[arickmann]

I've added a new version, plus an example plugin that implements the process.

This now does use a hook, registered when the plugin is activated.

The uninstall option is only offered if that hook has been registered. If the plugin is in the process of being uninstalled then the plugin is loaded, with the code being ringfenced by a function that checks to see if it is active (is_plugin_active).

The ringfencing code is placed at the top of the plugin so only the uninstall functions are exposed.

[darkdragon]

Thanks man for taking ownership of this ticket. The work you have done so far is totally awesome.

However, I would very much rather it was part of the core and not a plugin. Dude, a plugin... that was included with WordPress! Awesome!

[arickmann]

I think I have boiled it down to the essential processes now so have included a patch for three files within the core.

The processes work like this:

The plugin author can include the following function as part of their activation process:

register_plugin_assets( FILE , callback or NULL , array of database table names, array of WordPress option names );

When a plugin is deactivated, the plugins page will check to see if any assets have been registered and if so will provide the option to uninstall the plugin.

If the user chooses to uninstall it, the plugin will be included, the callback function will be registered as an action (uninstall_pluginname) and the action will be called, allowing other plugins to hook into this uninstall process.

After the callback has run the delete_plugin_assets function will be called and will delete the database tables and options mentioned.

It will then redirect to the plugins page and display the success or failure message.

The uninstall option will not be available while the plugin is activated.

I reconsidered the consequences of just installing the plugin, vs requiring the plugin author to ringfence their uninstall callback and I think they are acceptable. No actions or filters will be called and the user will be redirected back to the plugins page after the uninstall process.

[arickmann]

Plugin functions from wp-admin/includes/plugin.php

[arickmann]

Plugin functions from wp-includes/plugin.php

[arickmann]

Plugins page from wp-admin/plugins.php

[darkdragon]

Looks good. So the register_plugin_assets() would go in the activation hook. I'm going to see about testing it.

[darkdragon]

Wow! Just tested the system. I really do think you could split up the callback, options, and tables into three functions. You can also check to see if one the tables removed are part of WordPress protected tables to prevent accidental or maliciously deleting WordPress tables.

I hope your latest patch is committed.

For your information, for the phpdoc, there is a dash between the function name and description.

[DD32]

The only thing i can see wrong, is that some plugins create dynamic options while running ie cache_<md5>, It'd be nice to be able to append these options to the asset list.

As for the admin page, It'd be nice that if activated plugins had a link to the Configuration page for itself, I'd nearly suggest another column for Configure/Uninstall(when activated/deactivated), but there may allready be too many columns :)

(Apologies if you allready know this next stuff)
When creating patches, Its best to create it from the base WordPress directory, That way all teh changes files are included into one diff.

If you use windows, then TortoiseSVN is the recomended software by most, it'll allow you to pick which files in the directories you want to include in the diff.

[darkdragon]

Well, the configure I think is in another ticket or the discussion was part of a different ticket. That would be nice too. Using a similar system like arickmann's, it might as well be possible.

Also, it is a little bit difficult to figure out if the system appears well with the update message (but that might just be a mute point).

If the functionality was split up, that it would allow for dynamically adding options.

[DD32]

Replying to darkdragon:

Well, the configure I think is in another ticket or the discussion was part of a different ticket. That would be nice too. Using a similar system like arickmann's, it might as well be possible.

I was just throwing it out into the discussion for alternate ways for the uninstall link to be shown.

#4498 (Add configuration page links to Plugins page)

[darkdragon]

Patch that allows for dynamically adding of options and tables. Full patch with all of other three patches

[darkdragon]

New patch implements my suggestions and allows for dynamically adding of options and tables to the already defined list of tables and options.

[darkdragon]

I think the link at the side would be good for those of us who have higher resolutions (like me) who have the extra space to give. Having what could be four options, might be a little bit excessive.

[DD32]

Looks good to me :)

One final suggestion, Perhaps the tables and options parametres of register_plugin_assets() should be switched, Its more likely a plugin would register options than tables(If they choose to use that function rather than the individual ones)

I think the link at the side would be good for those of us who have higher resolutions (like me) who have the extra space to give. Having what could be four options, might be a little bit excessive.

I got to thinking about that too, I'm not sure if theres any redesign happening for the plugins table, but IMO, Maybe it could be changed so that there's a single "Actions" column, with pipe(|) seperated commands which can be applied to that plugin(activate/deactivate, edit, uninstall, configure, etc) - If anyone thinks thats a good idea, i'll open a ticket for it later, But once again, I'm not sure on what the redesign has got in mind for it(the version could also be shown under the plugins name too.. that'd give a bit more space).

[darkdragon]

Replying to DD32:

Looks good to me :)

One final suggestion, Perhaps the tables and options parametres of register_plugin_assets() should be switched, Its more likely a plugin would register options than tables(If they choose to use that function rather than the individual ones)

I agree, so I'm going to do it.

I got to thinking about that too, I'm not sure if theres any redesign happening for the plugins table, but IMO, Maybe it could be changed so that there's a single "Actions" column, with pipe(|) seperated commands which can be applied to that plugin(activate/deactivate, edit, uninstall, configure, etc) - If anyone thinks thats a good idea, i'll open a ticket for it later, But once again, I'm not sure on what the redesign has got in mind for it(the version could also be shown under the plugins name too.. that'd give a bit more space).

That would be sweet! Depending on what the final pending design is. If this ticket and yours get in, I might just rewrite my plugin for 2.5 with these features, since well the plugin isn't exactly compatible with 2.3 and I need to upgrade WordPress before it is eventually hacked, which would be bad for me.

[darkdragon]

Switches $tables and $options parameters, based off of arickmann patches

[darkdragon]

Just need dev support.

[DD32]

Replying to DD32:

I'm not sure if theres any redesign happening for the plugins table, but IMO, Maybe it could be changed so that there's a single "Actions" column, with pipe(|) seperated commands which can be applied to that plugin(activate/deactivate, edit, uninstall, configure, etc)

See #5660

[arickmann]

Thanks for the guidance on patches ( I am using Tortoise but I'm new to it) and PHPDoc, I didn't know either of those things.

I wonder if there is a danger of putting uninstall too close to activate or edit in the table?

[codealsatian]

This is an awesome addition to WordPress! I do want to suggest an addition to the functionality.

I have a plugin that writes Post and Page meta options. It would be great if those options can also be dynamically added as plugin assets to be deleted during the uninstall.

[arickmann]

Replying to codealsatian:

This is an awesome addition to WordPress! I do want to suggest an addition to the functionality.

I have a plugin that writes Post and Page meta options. It would be great if those options can also be dynamically added as plugin assets to be deleted during the uninstall.

I think this may be more suited to the callback function. I'm not sure it is common enough to need specific code to handle it.

[strider72]

Just my 2 cents RE user interface:

1) Please DO NOT add an "are you sure?" of any kind to deactivation. If a plugin author wants to put an "are you sure" or whatever on deactivate, he can already do that with the existing deactivation hook.

2) Please _do_ include an "are you sure?" with uninstall

3) Probably best if "uninstall" only appears on plugins page when the plugin is _not_ active -- i.e. if plugin is active, "uninstall" is simply not there -- this will avoid accidental clicks.

Sorry if I repeat things already done -- just saw this ticket and don't have chance to test now.

[DD32]

See Also: #7091 (Bulk managing of plugins & Plugin deletion)

[DD32]

See Also: #7091 (Bulk managing of plugins & Plugin deletion)

[jacobsantos]

Andrew,

Do you mind if the first patch is simple, with focus on implementing a simple check for whether the user hooks into the uninstall action for that plugin? I would like the ability to register assets and allow WordPress to automatically uninstall it for me.

However, I think that only advanced users will do that. If a user is going to install the plugin, then I think they'll know enough to put all of the uninstall stuff in the uninstall hook.

[jacobsantos]

Initial uninstall hook support

[jacobsantos]

Initial uninstall hook support adds uninstall link and uninstall button to active plugins. The issue with the uninstall is that it needs to see if the hook exists and can't see it unless the plugin is active. It is then assumed that if the user just wants to deactivate the plugin then the uninstall won't exist.

It is possible to create another option for plugins who can be uninstalled, when they are activated. This will allow for the plugins to be uninstalled. It will require that the plugin be activated (without the activate hook being called) and then have the hook ran.

[strider72]

I agree with Jacob that it should be there when plugin is _not_ activated. In fact, I think the "uninstall" link should _ONLY_ be there when the plugin is not activated.

In order for that to work, the uninstall function would have to be a separate file in the plugin directory -- in essence a separate mini plugin of its own.

This is because we _can't_ load the main plugin file to check for an uninstall, because many plugins run code simply when the file is loaded, and we don't want that if it's not activated.

So... I suggest that we standardize a file inside the plugin folder named "uninstall.php". The uninstall link is added to the Plugin "Action" column if three criteria have been met:

1) The plugin is not currently activated.
2) The uninstall.php file exists.
3) At some point while it was activated, the plugin registered itself as having an uninstall function (this prevents us for having uninstall functions for plugins that haven't added anything to the DB).

Other notes:

• The registration noted in criteria 3 would have to be persistent even after the plugin is deactivated.

[santosj]

Yeah, the original patch did that, it allowed you to register "assets" which I think I is like what you propose. I think checking for the file could be a good idea for whether the plugin can be uninstalled instead of checking for a function.

[santosj]

So basically register_uninstall_hook() really just updates an WordPress option that allows for the function to be called later. It is possible (through a major regex jujitsu) to pull the function and eval() it from the file.

[strider72]

I wouldn't regex it from the file. Only register it if the plugin actively registers it -- not just because uninstall.php exists.

Why?

1) We don't need an uninstall for a plugin that was never activated.
2) A plugin may or may not do something to the database, depending on how it's used. The author may only register it after it does X to the database.

So... register_uninstall_hook() should set WP to show the "uninstall" link if <a href="#comment:33">criteria 1 and 2</a> are met (i.e. uninstall.php exists and the plugin is not activated). Clicking the uninstall link should simply load that plugin's uninstall.php.

Or do we want to do a lot of abstracting and only have authors pass option names and (maybe) table names into new core WP functions? If we want to do the abstraction, we need to account for a lot of possibilities (records added to tables, columns added to tables, tables added to DB....)

[santosj]

No on the abstracting. If that is the case, then Andrew's patch would be fine and should be committed. I think it would be easier to get a simple patch which supports the uninstall hook than Andrew's implementation.

I'll create a patch which implement your suggestions, Strider72, because I think they are good.

[santosj]

Implements Strider's suggestions with support for uninstall.php and only display uninstall when deactivated

[santosj]

Latest patch needs testing. It supports both the uninstall.php and the uninstall hook.

[santosj]

I'm not sure if this is going to get in 2.7, it would be nice if it could, but it needs testing.

[santosj]

Going to upload a test plugin later.

[ryan]

Why not do uninstall as part of delete? Who wants to "uninstall" a plugin but leave it laying around. Just delete it. Uninstall is just a cleanup step when deleting.

[santosj]

I figure that the user might want to keep the plugin, but it makes sense to do it that way. I suppose that would be even easier. However, I might want to uninstall a plugin but keep the plugin files in case I want to activate the plugin again.

How about I add the uninstall as part of the delete as well as the uninstall button? Would that satisfy the requirements enough to have this be committed? Or is only on delete or nothing?

[ryan]

How about I add the uninstall as part of the delete as well as the uninstall button? Would that satisfy the requirements enough to have this be committed? Or is only on delete or nothing?

I'm imagining the support questions about what the heck the difference between Delete and Uninstall are. I think it's such an edge case that it's not worth having an uninstall button.

[santosj]

Calls uninstall hook on delete plugin

[strider72]

Replying to ryan:

I'm imagining the support questions about what the heck the difference between Delete and Uninstall are. I think it's such an edge case that it's not worth having an uninstall button.

Maybe the two should be combined. Have a link that says "Uninstall", and when you click it WP says "This will clear all settings and data for this plugin. Would you also like to delete the plugin files? Yes/No"

I'm not familiar with the Delete function. How exactly is that going to work? (Link please?)

[santosj]

Well, it is a mute point since the hook doesn't work.

[santosj]

Now it works, uploading new patch soon.

[santosj]

Adds uninstall hook to delete process, with support for hook and uninstall file.

[santosj]

Patch correctly implements uninstall hook for delete plugin process. Allows for both the uninstall hook and for an uninstall file.

[santosj]

Can this get into 2.7? It does need more testers.

[ryan]

(In [8585]) Plugin uninstall hooks from santosj. fixes #5625

[santosj]

Set @since phpdoc tags. Based off of r8586

[santosj]

Still need to set the @since tags for the new functions.

Thanks for committing!

[ryan]

(In [8587]) Update since tags. Props santosj. fixes #5625

[strider72]

The "We're going to delete all your files" page should also warn that all options, etc. are going to be deleted. Otherwise this might be unexpected behavior. (I can imagine somebody deleting a plugin that is corrupted, with the intent of reinstalling it. They may not want to delete the settings/data.)

Personally, I think we should allow, on that page, an option to delete but _not_ run the uninstall.

Otherwise, nice addition. (If I do say so myself ;-) )