Plugin bulk deletion attempts to define WP_UNINSTALL_PLUGIN constant multiple times

[jdgrimes]

The WP_UNINSTALL_PLUGIN constant is defined by uninstall_plugin() before the plugin's uninstall.php file is included (if the plugin has one). When deleting multiple plugins with uninstall.php files, the function attempts to define this constant each time, which will result in a notice.

PHP Notice: Constant WP_UNINSTALL_PLUGIN already defined

The notice will never be noticed by most users, because it will be silenced by default, but this could be an issue if plugins are checking for the value of WP_UNINSTALL_PLUGIN, as recommended ​here (which is linked to ​from the codex), and not just whether it is defined.

I don't have a good solution, sorry.

[jdgrimes]

One possibility would be to start just defining WP_UNINSTALL_PLUGIN as true if it isn't already defined, but that won't be backward compatible.

[nacin]

WP_UNINSTALL_PLUGIN is simply there so a plugin's uninstall.php file can confirm it was indeed invoked from the uninstall routine. We simply need to do if ( ! defined( 'WP_UNINSTALL_PLUGIN' ) ) defined( 'WP_UNINSTALL_PLUGIN', true ); It'd only avoid the notice, not actually change what's happening.

[nacin]

Sorry for the noise.

[jdgrimes]

Replying to nacin:

WP_UNINSTALL_PLUGIN is simply there so a plugin's uninstall.php file can confirm it was indeed invoked from the uninstall routine. We simply need to do if ( ! defined( 'WP_UNINSTALL_PLUGIN' ) ) defined( 'WP_UNINSTALL_PLUGIN', true ); It'd only avoid the notice, not actually change what's happening.

Right, that seems like the logical solution. The problem is that it could cause some plugins not to uninstall themselves, because it's not 100% backward compatible. I mentioned that in the original post, but I now see that I really didn't make it clear. Some people are checking it like this:

if ( __FILE__ != WP_UNINSTALL_PLUGIN )
     return;

Because right now the value of WP_UNINSTALL_PLUGIN is the path of the plugin's main file. If we just change it to true, the plugin won't uninstall. Unfortunately, this might be fairly ubiquitous (see the original post).

[spmlucas]

26735.diff simply adds the if statement to prevent the error. I've left the declaration of WP_UNINSTALL_PLUGIN alone, but I don't think changing it to true is a big deal. The codex prescribes simply checking for the existence of the constant, not the value - checking the value during a bulk uninstall is going to fail unless the plugin happens to be the first in the batch.

[jdgrimes]

Replying to jdgrimes:

The problem is that it could cause some plugins not to uninstall themselves, because it's not 100% backward compatible. I mentioned that in the original post, but I now see that I really didn't make it clear. Some people are checking it like this:

if ( __FILE__ != WP_UNINSTALL_PLUGIN )
     return;

Because right now the value of WP_UNINSTALL_PLUGIN is the path of the plugin's main file. If we just change it to true, the plugin won't uninstall. Unfortunately, this might be fairly ubiquitous (see the original post).

Actually, forget about this. I just realized that this will actually be backward compatible if we just define WP_UNINSTALL_PLUGIN as true. Because __FILE__ == true. So, as long as they aren't use strict typechecking (!==), they should be fine.

[jdgrimes]

Define WP_UNINSTALL_PLUGIN as true

[jdgrimes]

Replying to spmlucas:

26735.diff simply adds the if statement to prevent the error. I've left the declaration of WP_UNINSTALL_PLUGIN alone, but I don't think changing it to true is a big deal. The codex prescribes simply checking for the existence of the constant, not the value - checking the value during a bulk uninstall is going to fail unless the plugin happens to be the first in the batch.

That is true, but it is being done, and possibly by a lot of developers. Although the value isn't checked in the codex examples, one of the ​reference links there does prescribe this (as I pointed out in the original post). The fact that it is defined to the file path is misleading. One naturally assumes that it is OK to check the value. I believe it was originally intended that the value always reflect the plugin being uninstalled. Changing it to true would make it so those plugin's that are checking the value (and thought they were doing it right) would uninstall properly. I don't know of any case where it is necessary to check which plugin is being uninstalled, that would be broken by this.

[nacin]

No good solution here, punting.

An uninstall routine really just needs to lead with defined( 'WP_UNINSTALL_PLUGIN' ) ) or exit;. I didn't even know until this ticket it was defined as __FILE__.

[jdgrimes]

#41838 was marked as a duplicate.

[kkmuffme]

#36704 was marked as a duplicate.

[kkmuffme]

#44884 was marked as a duplicate.