Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#11850 closed enhancement (fixed)

Add plugin uninstaller notification text

Reported by: cyberhobo Owned by: nacin
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch tested commit
Focuses: Cc:


I'd like to see some indication in the plugin delete interface that a plugin has an uninstaller, and so will probably delete its data.

This is similar to issue #8783, but instead of proposing any new functionality, I just want to add some display information. I also have a patch.

The scenario that concerns me is this: a user has a plugin zip and wants to upgrade a plugin. They're not FTP savvy, so they try to use the upload interface, and get the "Destination folder already exists" message. So, they go back to the plugin listing and click "Delete" by the current plugin. It warns them that the files will be deleted, but this happens with automatic upgrades anyway, so it doesn't sound that scary. They click "Yes, delete these files", and their data is gone too...

Attachments (3)

test_uninstall_hook.php (473 bytes) - added by cyberhobo 6 years ago.
A test plugin that registers an uninstall hook.
plugins.diff (2.3 KB) - added by cyberhobo 6 years ago.
Patch freshened for r13230.
11850.diff (786 bytes) - added by scribu 6 years ago.
replace repetitive expression

Download all attachments as: .zip

Change History (22)

#1 @sc0ttkclark
6 years ago

I concur with this! +1

#2 @scribu
6 years ago

  • Milestone changed from Unassigned to 3.0

+1 on this.

I would replace

will also uninstall its data


will also delete its data

Also, I think we should open a new ticket for letting users upgrade plugins using the admin interface.

#3 @cyberhobo
6 years ago

  • Cc cyberhobo@… added

#4 @scribu
6 years ago

The patch works. However, is_uninstallable_plugin() only works for plugins that have an uninstall.php file.

It would be good if it detected if any callbacks are registered with register_uninstall_hook().

#5 @scribu
6 years ago

PS: Try to make patches relative to the wp root directory, not a subdirectory.

svn diff wp-admin/plugins.php

This saves a step for testers and commiters.

6 years ago

A test plugin that registers an uninstall hook.

#6 @cyberhobo
6 years ago

Thanks for the tip, I've updated my patch.

I can't confirm your statement about is_uninstallable_plugin(), though. The attached plugin registers an uninstall hook. I found that if the plugin is never activated, is_uninstallable_plugin() returns false for it, and the hook is not run on deletion. After it is activated and deactivated, is_uninstallable_plugin() returns true for it and the hook is executed on deletion.

Is that a bug for a separate ticket?

#7 @scribu
6 years ago

  • Keywords tested added; dev-feedback needs-testing removed

You're right. I hadn't activated the plugin.

One last thing: I think it would be good to make the notice more well... noticeable.

I was thinking of putting that text in ( ).

#8 @cyberhobo
6 years ago

I wondered about that too - it's hard to compete with both STRONG and EM text already on the line. I think another STRONG doesn't hurt too much though. Updating the patch with:

'<strong>%s</strong> by <em>%s</em> (will also <strong>delete its data</strong>)'

#9 @scribu
6 years ago

  • Keywords commit added

#11 @scribu
6 years ago

  • Keywords needs-patch added; has-patch tested commit removed

Patch got stale (probably due to MU merge).

6 years ago

Patch freshened for r13230.

#12 @cyberhobo
6 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

Works for me.

#13 @scribu
6 years ago

  • Keywords tested added; needs-testing removed

Also works here.

#14 @scribu
6 years ago

  • Keywords commit added

#15 @nacin
6 years ago

  • Owner changed from westi to nacin
  • Status changed from new to reviewing

#16 @nacin
6 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

(In [13470]) When deleting plugins, check for uninstall hooks and warn of data deletion. Props cyberhobo. Pluralize some string(s). fixes #11850

#17 @nacin
6 years ago

This looked really good. I adjusted some of the other strings as well and got rid of the Delete Plugin(s)-type strings by adding some pluralization checks. (And poked fun at that in the changeset message.)

6 years ago

replace repetitive expression

#18 @scribu
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Cool :)

Related to strings, this sounds off:

'Deleting the selected plugins will remove the following plugins and its files:'

How about:

'You are about to remove the following plugins:'

See 11850.diff

#19 @nacin
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [13477]) Clean up repetitive string. fixes #11850, props scribu

Note: See TracTickets for help on using tickets.