WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#17833 closed defect (bug) (fixed)

Auto update plugins does not activate activation hooks

Reported by: opajaap Owned by: ryan
Milestone: 3.4 Priority: normal
Severity: minor Version: 3.1.3
Component: Upgrade/Install Keywords: has-patch ux-feedback
Focuses: Cc:

Description

On automatic bulk update of plugins, the plugins are not de-activated and activated after update. So actions that are needed to be done that belong in the activation hook like running dbdelta are not executed.

This causes inconsistencies and therefor errors in the plugin.

Attachments (4)

update.diff (830 bytes) - added by toppa 2 years ago.
This is for wp-admin/update.php (not wp-admin/includes/update.php
class-wp-upgrader.diff (1.4 KB) - added by toppa 2 years ago.
plugin-auto-update-messages.patch (2.2 KB) - added by toppa 2 years ago.
17833.diff (1.6 KB) - added by ryan 2 years ago.
Refresh

Download all attachments as: .zip

Change History (28)

comment:1 opajaap3 years ago

  • Cc Opajaap@… added

comment:2 dd323 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

This has been a design choice. In the past, the activation hooks ran but not the deactivation hooks (as too many plugins would remove data on deactivation).

The suggested method is to utilise a 'plugin_version' option, and upgrade any options, and database tables when plugin_version != Hardcoded version in plugin file (And do it when is_Admin() or similar).

See #14915

comment:3 johnbillion3 years ago

Another way of thinking about this is, if the plugin is manually updated then the activation hook will not be fired then either. This means all updates (either automatic or manual) are consistent and your plugin should do as DD32 said with regard to update routines.

comment:4 opajaap3 years ago

Ok i do it now with an add_action('admin_init', 'my_setup'); and a compare between a saved and new version number.This works.
It still is confusing that the manual update process says 'Plugin de-activated' and later 'Plugin activated successfully'. Maybe change it in 'switched on' and 'off'?

toppa2 years ago

This is for wp-admin/update.php (not wp-admin/includes/update.php

toppa2 years ago

comment:5 toppa2 years ago

  • Resolution wontfix deleted
  • Status changed from closed to reopened

The two patches I've uploaded change the wording when auto-updating a plugin to "turn on" and "turn off" instead of activate and deactivate. It's misleading that the word "activate" has a specific meaning and associated functionality on the main plugin panel that does not apply to the use of the same word when you auto-update a plugin. As a plugin author I found this confusing until I found this trac ticket.

Last edited 2 years ago by toppa (previous) (diff)

comment:6 toppa2 years ago

  • Version changed from 3.1.3 to 3.3.1

comment:7 scribu2 years ago

  • Keywords has-patch ux-feedback added
  • Milestone set to Awaiting Review

You should upload a single diff with both changes.

comment:8 scribu2 years ago

  • Keywords close added; ux-feedback removed

Frankly, I don't think end-users should be burdened with this knowledge. They shouldn't care about activation hooks. It's up to the developers to know this stuff and make their plugins Just Work (tm).

comment:9 toppa2 years ago

Thanks for the quick reply scribu. I don't intend it as anything extra for end users to deal with. I'm simply trying to reduce ambiguity about what is happening during the auto-update. The intent is to make it that much easier for plugin authors to be aware of what does and doesn't happen during auto-updates, so they are more likely to write code that Just Works.

comment:10 williamsba12 years ago

  • Keywords dev-feedback added

I agree with Toppa here. Yes plugin devs should know this stuff, but terms like "activate" and "deactivate" for plugins should only be used when that is actually what's happening. In this case it's not, so I can see how users (and even plugin devs) could easily be confused.

comment:11 scribu2 years ago

  • Keywords ux-feedback added; close dev-feedback removed

Actually, why do we need to be so verbose? Unlike downloading a zip, plugin de/activation happens instantly (especially when de/activation hooks are not run), so let's just show a message only when something goes wrong.

comment:12 toppa2 years ago

That's a great suggestion. I've uploaded a patch that leaves out the de/activation messages unless there is a problem.

comment:13 scribu2 years ago

  • Keywords ux-feedback removed
  • Milestone changed from Awaiting Review to 3.4

Here's what the output looks like for the bulk updates:

Downloading update from http://downloads.wordpress.org/plugin/debug-bar.0.7.zip…
Unpacking the update…
Installing the latest version…
Removing the old version of the plugin…
Plugin updated successfully.

It should be the same for single updates.

Last edited 2 years ago by scribu (previous) (diff)

comment:14 toppa2 years ago

When I do a bulk update with my patch, I get this:

The update process is starting. This process may take a while on some hosts, so please be patient.

Enabling Maintenance mode…

Updating Plugin Shashin (1/2)
Shashin updated successfully. Show Details.

Updating Plugin Toppa Plugins Libraries for WordPress (2/2)
Toppa Plugins Libraries for WordPress updated successfully. Show Details.

Disabling Maintenance mode…

All updates have been completed.

When I do a single update I get this:

Downloading update from http://downloads.wordpress.org/plugin/shashin.3.1.4.zip…

Unpacking the update…

Installing the latest version…

Removing the old version of the plugin…

Plugin updated successfully.

It looks like you pasted a single update message, not a bulk one. Do you mean that you want the unpacking, installing, etc., messages to show when its a bulk update? Or do you mean you when it's a single update, you want it to just say "x updated successfully" like the bulk update does?

comment:15 scribu2 years ago

I was pasting the content you get when you click the "Show Details" link.

So, if the message is the same for both single and bulk updates, when exactly is the 'Deactivating the plugin…' message shown?

comment:16 scribu2 years ago

  • Keywords ux-feedback added

Right... it's only shown when the plugin is currently active. That's what happens when you're too lazy to look at the code.

So the question remains: is it a good idea to a) remove those messages altogether or b) just change the wording or c) leave it as is?

comment:17 toppa2 years ago

Removing them was my interpretation of your suggestion to make the process less verbose ;-) I like removing them because it means neither users nor plugin devs will think the plugin's de/activation functions are being called during the auto-update (as happens with the de/activation links on the plugin panel). That seems tidier than trying to find some subtle wording to distinguish the two different flavors of de/activation. Appropriate messages still show if there is an error.

comment:18 TechStudioWP2 years ago

  • Cc TechStudioWP added
  • Severity changed from normal to minor
  • Type changed from defect (bug) to feature request

I am not an especially experienced developer, so I hope my comments are not ignorant or misunderstood.

It seems completely logical that the activation and deactivation hooks not be triggered during automatic update. It could cause a lot of unintended side effects which has already been made clear. A solution involving a plugin developer internally tracking version was mentioned by dd32 that will work perfectly for what I think most of us watching this thread really need and I am satisfied to take his advice; however, would it not be widely useful for WordPress to track plugin versions and have an associated function or two related to this? This would allow us to have a hook we can use when automatic updates occur. Many other uses would arise as certain actions could be called when particular versions are surpassed during updates.

If these comments are poorly expressed or off-topic due to a potential misunderstanding, feel free to remove them.

Version 0, edited 2 years ago by TechStudioWP (next)

comment:19 nacin2 years ago

  • Type changed from feature request to defect (bug)

You can find discussion on that in #14912.

Moving away from feature request as the issue is strings.

comment:20 TechStudioWP2 years ago

Thanks. I see that what I mentioned is already being discussed at length.

comment:21 williamsba12 years ago

+1 to removing the message all together

comment:22 SergeyBiryukov2 years ago

  • Version changed from 3.3.1 to 3.1.3

Version number indicates when the bug was initially introduced/reported.

comment:23 nacin2 years ago

The changes in wp-admin/includes/class-wp-upgrader.php make sense -- removing the messaging.

I think we should probably leave the iframe code for now. Problem is, if the plugin fails to reactivate, it's going to show an error stating as such. And if we're going to do this iframe loading sandbox thing, we should probably have a success message.

ryan2 years ago

Refresh

comment:24 ryan2 years ago

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

In [20786]:

Remove activate/deactivate messages from plugin upgrades. Props toppa. fixes #17833

Note: See TracTickets for help on using tickets.