deactivated_plugin behaves improperly

[wpsmith]

Currently, if someone were to hook into deactivated_plugin, one should expect that the $plugin actually be deactivated.

So if, for example, I hook into it with the following code, deactivating Addthis, I don't get the expected behavior.

add_action( 'deactivated_plugin', 'dtat_deactivate_self', 10, 2 );
/**
 *  Deactivate ourself if Premise is deactivated.
 */
function dtat_deactivate_self( $plugin, $network_deactivating ) {
	if ( 'addthis/addthis_social_widget.php' == $plugin ) {
		die( 'Addthis: ' . print_r( is_plugin_active( $plugin ), 1 ) );
	}
}

The plugin still shows that it is active. So if I hook in here to check if plugin has been deactivated, then it fails. Instead, the deactivated_plugin hook should appear after the update_option call, which is where the plugin is actually deactivated.

OR, the docs are wrong and should be updated.

Attached is a sample addthis plugin extension that deactivates after Addthis is deactivated by being forced to use update_option_active_plugins and update_option_active_sitewide_plugins.

See ​Github Gist sample plugin for ​Addthis.

[wpsmith]

Option 2: Docs Update

[wpsmith]

Option 1 (preferable): move action

[wpsmith]

Related #27189.

[ocean90]

Option 1 seems ok. I would also move 'deactivate_' . $plugin and deactivate_plugin together.

[ocean90]

Related: #20241

[wpsmith]

Replying to ocean90:

Option 1 seems ok. I would also move 'deactivate_' . $plugin and deactivate_plugin together.

I don't think they should be moved together. The tense of 'deactivate_' . $plugin suggests that it is in the middle of deactivating the current plugin or is not yet complete in the deactivation process. It is where the current plugin being deactivated hooks to run it's deactivating process, function, or method. The later, 'deactivated_plugin', suggests that the plugin has already been deactivated. To me, these are two different steps in the process, even thou a deactivated plugin is only an option in the WordPress database.

[ocean90]

Replying to wpsmith:
I'm talking about deactivate_plugin without ed, see 27188.patch​.

[wpsmith]

Replying to SergeyBiryukov:
This is still slated for 3.9. Any movement on this since we are now in Beta?

[nacin]

This function works in a loop. The option is not actually updated until the end, while each plugin gets its hooks fired in turn. The patches attached won't work as they move deactivated_plugin to after the update_option() calls — from inside the loop to outside of it.

I agree, conceptually, that the option should be updated *before* deactivated_plugin. To do that, we'd need to call update_option() inside the loop, which means more database calls (at least when we're bulk deactivating). I don't see that as a terrible thing, but it's not something I'd like to just do (especially since we hadn't caught it before now that this change would be a problem).

activate_plugin() does *not* work in a loop. activate_plugins() calls activate_plugin() which handles both the hooks and the option calls. activated_plugin properly fires at the right time.

I don't really care if we introduce a deactivate_plugin() function to mimic activate_plugin(), but I'm otherwise fine with these changes if done properly.

In the process, I noticed some docs were wrong, so I'm fixing those as well.

[nacin]

In 27769:

Fix docs for hooks in deactivate_plugins() and activate_plugin(). see #27189, #27188.

[wpsmith]

Replying to ocean90:

Replying to wpsmith:
I'm talking about deactivate_plugin without ed, see 27188.patch​.

I think this is the correct way and expected way. I am attaching a refreshed updated version.

[wpsmith]

Refreshed