Make WordPress Core

Opened 14 years ago

Closed 14 years ago

#11850 closed enhancement (fixed)

Add plugin uninstaller notification text

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

Description

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 14 years ago.
A test plugin that registers an uninstall hook.
plugins.diff (2.3 KB) - added by cyberhobo 14 years ago.
Patch freshened for r13230.
11850.diff (786 bytes) - added by scribu 14 years ago.
replace repetitive expression

Download all attachments as: .zip

Change History (22)

#1 @sc0ttkclark
14 years ago

I concur with this! +1

#2 @scribu
14 years ago

  • Milestone changed from Unassigned to 3.0

+1 on this.

I would replace

will also uninstall its data

with

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
14 years ago

  • Cc cyberhobo@… added

#4 @scribu
14 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
14 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.

@cyberhobo
14 years ago

A test plugin that registers an uninstall hook.

#6 @cyberhobo
14 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
14 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
14 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
14 years ago

  • Keywords commit added

#11 @scribu
14 years ago

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

Patch got stale (probably due to MU merge).

@cyberhobo
14 years ago

Patch freshened for r13230.

#12 @cyberhobo
14 years ago

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

Works for me.

#13 @scribu
14 years ago

  • Keywords tested added; needs-testing removed

Also works here.

#14 @scribu
14 years ago

  • Keywords commit added

#15 @nacin
14 years ago

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

#16 @nacin
14 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
14 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.)

@scribu
14 years ago

replace repetitive expression

#18 @scribu
14 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
14 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.