Ticket #5625 (new enhancement)

Opened 4 months ago

Last modified 3 months ago

Include a separate uninstall for plugins

Reported by: arickmann Assigned to: anonymous
Priority: normal Milestone: 2.6
Component: Administration Version:
Severity: normal Keywords: plugin-management has-patch tested
Cc:

Description

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.

Attachments

fun_with_uninstallation.php (3.5 kB) - added by arickmann on 01/10/08 08:02:32.
Plugin that adds uninstallation capability.
fun_with_uninstallation0.2.php (5.6 kB) - added by arickmann on 01/11/08 20:47:01.
Version 0.2 that uses a hook.
sample_uninstallable_plugin.php (3.1 kB) - added by arickmann on 01/11/08 20:47:34.
A sample plugin that implements the uninstall process.
wp-admin_includes_plugin.php.diff (3.5 kB) - added by arickmann on 01/13/08 17:54:33.
Plugin functions from wp-admin/includes/plugin.php
wp-includes_plugin.php.diff (2.3 kB) - added by arickmann on 01/13/08 17:55:17.
Plugin functions from wp-includes/plugin.php
wp-admin_plugins.php.diff (2.9 kB) - added by arickmann on 01/13/08 17:56:06.
Plugins page from wp-admin/plugins.php
5625.r6603.diff (11.1 kB) - added by darkdragon on 01/13/08 22:45:38.
Patch that allows for dynamically adding of options and tables. Full patch with all of other three patches
5625.r6603.2.diff (11.1 kB) - added by darkdragon on 01/13/08 23:06:52.
Switches $tables and $options parameters, based off of arickmann patches

Change History

01/10/08 08:02:32 changed by arickmann

  • attachment fun_with_uninstallation.php added.

Plugin that adds uninstallation capability.

01/10/08 08:25:36 changed by 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.

01/10/08 13:47:50 changed by darkdragon

I agree with DD32, it should be a hook.

01/10/08 13:48:02 changed by darkdragon

  • keywords set to plugin-management.

(follow-up: ↓ 5 ) 01/10/08 17:34:43 changed by 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.

(in reply to: ↑ 4 ; follow-up: ↓ 6 ) 01/10/08 22:08:41 changed by 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.

(in reply to: ↑ 5 ) 01/10/08 23:55:38 changed by 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.

01/11/08 20:47:01 changed by arickmann

  • attachment fun_with_uninstallation0.2.php added.

Version 0.2 that uses a hook.

01/11/08 20:47:34 changed by arickmann

  • attachment sample_uninstallable_plugin.php added.

A sample plugin that implements the uninstall process.

01/11/08 20:50:08 changed by 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.

01/12/08 03:07:49 changed by 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!

01/13/08 17:53:32 changed by 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.

01/13/08 17:54:33 changed by arickmann

  • attachment wp-admin_includes_plugin.php.diff added.

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

01/13/08 17:55:17 changed by arickmann

  • attachment wp-includes_plugin.php.diff added.

Plugin functions from wp-includes/plugin.php

01/13/08 17:56:06 changed by arickmann

  • attachment wp-admin_plugins.php.diff added.

Plugins page from wp-admin/plugins.php

01/13/08 18:02:41 changed by arickmann

  • keywords changed from plugin-management to plugin-management has-patch.

01/13/08 20:57:05 changed by darkdragon

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

01/13/08 21:29:33 changed by darkdragon

  • keywords changed from plugin-management has-patch to plugin-management has-patch tested.

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.

01/13/08 21:55:01 changed by 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.

(follow-up: ↓ 15 ) 01/13/08 22:12:35 changed by 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.

(in reply to: ↑ 14 ) 01/13/08 22:23:26 changed by 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)

01/13/08 22:45:38 changed by darkdragon

  • attachment 5625.r6603.diff added.

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

01/13/08 22:52:00 changed by darkdragon

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

01/13/08 22:54:07 changed by 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.

(follow-ups: ↓ 19 ↓ 21 ) 01/13/08 23:00:37 changed by 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).

(in reply to: ↑ 18 ) 01/13/08 23:04:37 changed by 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.

01/13/08 23:06:52 changed by darkdragon

  • attachment 5625.r6603.2.diff added.

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

01/13/08 23:07:45 changed by darkdragon

Just need dev support.

(in reply to: ↑ 18 ) 01/14/08 01:12:28 changed by 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

01/14/08 17:27:21 changed by 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?

(follow-up: ↓ 24 ) 01/18/08 15:24:42 changed by 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.

(in reply to: ↑ 23 ) 02/11/08 07:41:09 changed by 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.