Allow Plugin/Theme updates from a uploaded .zip file.

[hakre]

Plugin administration lacks of an update possibility by uploading zip files. This feature is only half-done in 2.7, you can only upload plugins as zip to install them, not to update them.

Zip file uploads should be treated the same as the other install/update possibilities like remote.

#9708 provides a testcase that can be used to test such am update by zip.

currently a over-upload in the install page does throw an error that the directory already exists:

Installing Plugin from uploaded file: 9708-plugin-testcase-9708-plugin-a-v-0.2.zip

Unpacking the package.

Installing the plugin.

Destination folder already exists. [...]worpress-trunk/wp-content/plugins/9708-plugin-testcase/

Plugin Install Failed.

[DD32]

Moving to a Future release, If its so chosen to be integrated at some stage.

Personally. I'd prefer to leave it to plugin material (I may even write something for that).

If a trac admin could edit that description to remove the huge code block, that'd be nice.

[hakre]

If this is rated as plugin material then I would suggest to move the current remote installer in the plugin domain as well. I mean, it's more important to be able to install from zip then from third party remote even that is wordpress.org.

And for clearing the mess in the code example above: please, yes.

[DD32]

My reasoning was this:

• For Installs: You've only got a source to deal with, its a straight up install.
• For Upgrades: You're upgrading a specific plugin, It'd need to be set to select the plugin/theme to upgrade. The automatic upgrades are more suited to an automatic thing IMO. Theres nothing stopping plugins from adding their own update notifications + automatic upgrades.

Other items which such a plugin could implement include Installation from a remote user-specified URL. Currently it only allows uploaded files.

I personally see upgrades and Installations as vastly different things, A user might install plugins from multiple locations, WordPress.org, A remote URL(which WP doesnt support) or a plugin they've downloaded. Upgrades are different though, the user needs to be notified of an upgrade, and then the user would need to identify the plugin which the package is supposed to upgrade.

It'd be a simple patch though, If someones interested in making a UI for it (And a dev likes it) I'll write in the stuff for the backend-upgrade if need be (Only since i understand it pretty well, And realise some people might be better suited at UI programming)

[Denis-de-Bernardy]

just checking... can't the install screen already be used to upgrade?

[DD32]

No. Thus the error message which was provided above in the ticket.

[Denis-de-Bernardy]

thoughts about setting clear_destination to true in this case?

[DD32]

thoughts about setting clear_destination to true in this case?

a -1 from me there.

IMO zips should be trusted less than whats already on the system, Auto-installed Plugins have the catch that all folder names are unique, and therefor, dont conflict, but if i download a plugin called "something Cool by Denis" should it overwrite "Something cool by DD32" which i installed from elsewhere?

Another option for this, Is to wp_unique_filename() (well..dirname) on the dest. folder, That'd have the benefit of no conflicts, However, It does have the downside that the plugin upgrader (still) doesnt respect the plugin folders name (ie. it creates a new folder based on the zip name, or the .org slug in the .zip rather than using the current folder (eg. core-control installed in my-plugin-cc/ will be renamed))) - So that basically rules that out at this stage..

Maybe for uploaded plugins a unique_name.. else if its from .org.. not.. But that doesnt feel right to me, since it varies depending on the use-case

[Denis-de-Bernardy]

how about this: we check if the folder exists. If so, we validate that the new plugin files return similar data as the old one (i.e. same plugin name, higher version being uploaded, not necessarily same author since it may have been maintained by someone new). If true, it's pretty safe to set the clear_destination to true.

[DD32]

how about this:

Hm. Sounds like a good idea, Basically the same thing api.wordpress.org does behind the scene..

But if thats the case, Might also need a checkbox "This is an upgrade for ...." or something. Eitherway, Its a potential candidate for a future release.

(Marking as feature request until a patch comes in)

[Denis-de-Bernardy]

I mostly see it as a means to easily update a theme or a plugin that is *not* in the repository. :-)

[DD32]

I mostly see it as a means to easily update a theme or a plugin that is *not* in the repository. :-)

Yeah, Thats what i meant, BUT my point was that what your idea is, Is exactly what the API does for wordpress hosted plugins.

[hakre]

Additionally it would be nice that it's possible to upgarde the whole WordPress installation by providing a zipfile. Is somehow related to the original suggestion. Just overwriting every file that already exists to get a clean start-over.

[hakre]

Related: #10596, #11281

[Denis-de-Bernardy]

dup with patch: #11915

[cyberhobo]

The dup was mine. I updated my patch after reading this discussion to incorporate some extra checks. But more important, I think, it's an illustration with a bare minimum UI that I find helpful for getting a feel for the possible pros and cons of this feature.

My one addition to the need for the feature is related to #11850. Briefly, a user with a ZIP who fails to upgrade a plugin via upload might be tempted to delete the existing plugin, which might have an uninstaller that deletes their data.

So, without getting into the implementation, I wonder if this illustration can help us decide for or against this feature? I myself am for it, but I'd just like the bare minimum to address my above concern.

[scribu]

@cyberhobo: As long as the UX is identical to the normal update process, I don't see the problem.

+1 for this feature.

[scribu]

By the way, I think it wouldn't be that hard to allow multiple plugin/theme upgrades through a single .zip file.

Here's how it could work (for plugins):

1. Unzip archive.
2. Scan all extracted files for plugin headers.
3. Match any found plugins with the currently installed plugins.
4. Show a confirmation screen to the user about which plugins would be upgraded.
5. Do normal upgrade procedure for matched plugins.

[cyberhobo]

I'll see if I can find some time to look at this again.

Re: Step 4, I recall thinking it would be harder to add an extra exchange like a confirmation (in addition to being a UX change). My compromise was to add an "upgrade existing plugin(s)" checkbox to the initial upload form instead (also a UX change).

The other question I recall having is that I needed data like get_plugins() returns, but get_plugins() won't run on the unzipped archive in the working directory. I'm not entirely clear why that is, or if it would be worth adapting that code to this purpose rather than repeating much of it.

[scribu]

Replying to cyberhobo:

The other question I recall having is that I needed data like get_plugins() returns, but get_plugins() won't run on the unzipped archive in the working directory. I'm not entirely clear why that is, or if it would be worth adapting that code to this purpose rather than repeating much of it.

Better to make get_plugins() more general than have duplicate code.

[hakre]

Related: #16923

[cyberhobo]

Refresh, improvements

[cyberhobo]

Got some work done at WordCamp Reno today:

• updated to current trunk - still works!

• modified get_plugins() to take an alternate plugin root. I skip caching in that case because it doesn't seem likely to be done more than once.

• doesn't do multiple upgrades from a zip, but that still seems within reach

[SergeyBiryukov]

Related: #27196

[swissspidy]

See also #20578

[countrymusicchicago]

This would be a huge time saver.

[Doobeedoo]

7 years ago, it's still impossible to update a plugin with the .zip ?

[pingram3541]

+1 on this. Still very much a painful process to update plugins not in repo or don't have their own update mechanism, not every site has wp-cli and its just that much more work added to our lives to move from already being logged into the back end to using a third party client to update plugins. I'm actually surprised a plugin for this doesn't already exist =)

[bfintal]

Perhaps we can merge parts of this plugin into core: ​https://wordpress.org/plugins/easy-theme-and-plugin-upgrades/

[ocean90]

#38139 was marked as a duplicate.

[kda406]

I cannot believe this is not already in the core, and hasn't been added for all these years. This would be a huge time and bandwidth saver. No need to download the same plugins for every one of my sites over and over.

[nimmolo]

+1 for add to core. The plugin mentioned by @bfintal works great, but this seems like a necessary core feature. Sites hosted on servers without FTP access are too much of a chore to update.

[lukecavanagh]

Seems like a solid core feature for 4.8.

[rinkuyadav999]

+1 for add to core.

[pcpro178]

+1 for add to core.

[zubaka]

+1 to add this feature to core

[swissspidy]

#40561 was marked as a duplicate.

[folletto]

+1 to this.

I'd also add that the lack of this feature makes impossible to update an active theme without disabling it first and deleting it, and disabling a theme means creating a gap for the site, plus it might lose customizations settings (widgets dropped / moved to a different sidebar).

If a user doesn't have SSH/SFTP access and wants to update an active theme, the lack of this feature makes it impossible without disrupting the site.

[ahortin]

Good to see this getting some traction after 8 years. This would be a huge time-saver, not to mention how much more convenient it would make updating themes & plugins. It's ridiculous that it's not already in core.

[theMikeD]

+1 to this. Is anyone "owning" this feature?

[nimmolo]

@rockwell15 PLEASE can we schedule this for the next point version. (Plugin and theme updates from .zip upload)

[swissspidy]

@nimmolo Unfortunately it's not that easy.

This ticket definitely needs a refreshed patch and lots of testing (perhaps even a unit test).

If anyone wants to work on this, feel free. Right now, this doesn't have priority though as it's not one of the current focuses of the project.

[kda406]

@swissspidy You are missing the point. Many people need this and have been requesting it for 8 years now. Enhancements - no I'm going to call this a fix because it should have been in there from the beginning. Fixes with wide spread need which have been long awaited, should have developer precedence.

When a new version of WordPress comes out, I always read to see if they have fixed this problem yet. Then I am disappointed when I learn they have wasted precious coding time and resources for unimportant "improvements".

Security related patches are the only things that should have higher priority over widely needed, log awaited fixes like this one.

[swissspidy]

See also #24579.

[melchoyce]

Proposed workflow:

• Drag and drop an updated zip file for the theme you're updated directly onto themes.php. Get an Shiny Updates notice on the theme being updated. Boom, updated!
• Also, add some sort of "update theme from zip" button to the theme details modal.

I'll sketch out a flow for this.

[pingram3541]

So excited this is finally getting traction! I just wanted to add that there is also great value in the ability to also roll back a version via zip upload as well, not just update so maybe this can be considered as well?

Also, should there be any consideration for retention of what is overwritten? A download link?

[FolioVision]

+1

Reduces dependency on WordPress.org plugin directory and at this point that's a good thing. As plugin developers in an area of constant change (video), we are regularly asking customers to delete the existing plugin to install a custom beta version. Zip updates would definitely make customers lives easier.

As swisspidy notes, it's harder than it looks although the code to do updates right must exist on WP.org's server and could be made to work locally.

[ronalfy]

Definitely a +1 here.

We recently ran into this situation with a client site and the only workaround was renaming the folder and deleting the old theme.

[StephenCronin]

+1 for this.

It seems to be quite common that real users try to update using the zip upload (install) feature and therefore get the "destination folder already exists" message.

If you do a ​Google search, you'll see there are a bunch of articles about this, including FAQs from theme and plugin authors, which shows they get lots of users asking their support teams about this. Likewise there are ​a bunch of topics on the Support Forums.

At minimum we should warn users that this won't work before they attempt it. Ideally, we'd make it work, or come up with a solution such as the one Mel proposed.

All those articles tell users to update via FTP and although that will work, I'm sure core can offer a better user experience than that.

[melchoyce]

Relevant Twitter conversation:

​https://twitter.com/andrea_r/status/879779357878886401

​https://twitter.com/andrea_r/status/879841515010445313

(I wish these embedded...)

I definitely think we should allow both uploading new themes, and updating existing themes, via drag-and-drop (#24579), but maybe we don't need to add new UI for updating — maybe having people go through the existing upload flow is fine.

[rinkuyadav999]

Too much +1 for this :)

[folletto]

I agree, the drag'n'drop flow is going to be useful regardless of the specific flow as it's a speed/advanced feature, and we can start by allowing update with the same UI for the rest, and iterate later if we prefer to improve the design further.

[ahortin]

The ability to upload/update an existing theme & plugin, even using the existing workflow would be a million times better than not being able to do it, which is what we have now.

So excited that this is finally getting some traction! *Squeeeee!*

[alex-ye]

It would be nice feature to add into the core.
I had built those two simple plugins 1 year ago, hope it helps. :)
​https://wordpress.org/plugins/reinstall-themes/
​https://wordpress.org/plugins/reinstall-plugins/

[pento]

Soooo... this ticket hasn't made much progress this cycle. :-)

Unfortunately, the people who would usually work on these things (@dd32 and I, for example) are unavailable until late September, and that's way too late to be starting work on this feature. There are a lot of hidden complexities and edge cases to consider, it requires a deep dive into the update code, and I'd really like to see masses of unit tests (and maybe even some integration tests).

If someone can pick this up and run with it for the next 4 weeks, I can absolutely provide reviews and feedback until I'm available to join you more regularly. We have just under 6 weeks until the feature merge deadline, that should be sufficient to build this, and give it time to soak. If doesn't have any movement until late September, it almost certainly won't make it into 4.9.

If you're thinking about jumping in, but aren't sure where to start, here are some steps to think about.

• Go through the install/upgrade related screens many times, so you know exactly how it currently works.
• Take screen shots, post them to ​make/test, link the post here.
• Write about exactly how you think the UX flow should work. We have plenty of UX experts subscribed to this ticket who can give you feedback.
• Iterate on the flow. Think about the edge cases - what will make it weird? What will make it bad? What will make it break?
• Write a prototype. A plugin would be easier to test, but harder to merge - don't worry about that now.
• Post more screenshots and video to make/test.
• Iterate.

If any of these aren't your strong point, don't sweat it - there are a bunch of people here who will help you. If you can only dedicate time to part of it, that's cool! There are a bunch of people here who can join you to keep things moving.

If the ticket gets this far, it's on track to get it merged in 4.9! Good work, friends! You're awesome people. :-D

[dd32]

As @pento said above, I'll review any attempts made here - but with the caveat that I can't do so until the last week of September at the earliest.
If this were a super-simple thing to add, I'd have added it a long time ago, unfortunately there are a lot of edge-cases that are not immediately obvious - IIRC it's not merely 'connector' code required, but something a lot deeper.

The update code also differs significantly from the install code - Install handles various archive structures and is far more forgiving than the update handler.
For example, currently updates require the ZIP file structure match *exactly* what W.org produces - however most non-W.org ZIP files do not match in every respect, and causes problems for 3rd-party updates occasionally (until they get the structure "right"). The installer code handles the various archive structures a lot better, but still gets complaints sometimes (for example, ending up with a directory of wp-content/plugins/plugin-name.0.1.2.3.tmp/ under certain circumstances)

IMHO: Any change here will need to touch on the main update handlers to avoid code duplication, and the changes there will need deep testing, as there's no current unit testing, integration testing, or any other kind of testing in place there for any install/update actions.

(Some of the comments from 4+ years here will be completely irrelevant to any effort too, take them with a grain of salt, but verify any limitations mentioned)

[JoshuaWold]

In order to catch myself up on the problem that's happening here, I've created a user flow with screenshots: ​https://make.wordpress.org/test/2017/08/27/updating-plugins-with-zip-uploads/.

It took a while to understand the issue, so hopefully these screenshots will help others.

[dnavarrojr]

Ugh, getting punted again...

[psykro]

@pento @dd32 I'm keen to see if I can either own this, or do as much as possible in the next four weeks to push it forward. At this point I have some questions

1. Does it make sense to start with only making this work if the user uploads an updated plugin zip from the Plugin update screen? There has been some discussion around implementing drag/drop functionality but that seems to me to be a good feature for a future release.
2. The current process only checks for a matching folder structure, is this enough of a check? My thought is that it could do the following to determine whether to upgrade or install
   • check for a matching folder
   • if not ground check for an installed plugin with the same Plugin Name
3. Does the upgrade process need to be more complicated than
   • delete old plugin files
   • extract new plugins files

[Clorith]

It should be more complicated than that, yes.

One thing is if you hit an "invalid" structure, as mentioned in a previous comment.

My concern that needs ot be addressed is when you take into consideration things such as like folder name (slug) matching. Someone made my-plugin as the slug for My first plugin, but there's already a my-plugin used by the Mittens by York Plugincompany who prefixed their plugin with my followed by plugin as the slug.

What happens if I accidentally grab an older version and upload it, should it automatically replace everything, should I be notified that I'm about to perform a downgrade (I think I should be).

[psykro]

@Clorith thanks for the feedback, noted regarding uploading older versions.

My question around the actual upgrade process was more along the lines of 'WordPress has already determined (by means to still be discussed) that this plugin can be upgraded'.

So once the something like 'can_plugin_be_updated()' returns true, does the physical action of upgrading the plugin need to be more complicated than something like

• rename current plugin directory
• upload and unzip new plugin
• delete renamed plugin directory

I'm still going to follow @pento's suggestions of diving deep into the code and taking screenshots, but I'm just clarifying some initial questions/suggestions that pop into my head.

In my mind the checks for upgrading a plugin should be a combination of the plugin slug and Plugin header meta info for example

• Plugin name
• Version
• Text Domain

However I realise that the only field that a plugin actually requires is the Plugin Name

My initial feeling is that if the plugin directory name and Plugin Name meta match, consider the plugin to be upgraded. If the plugin has a valid Version field, check against that, if not warn the user of a possible 'downgrade'.

[joyously]

Replying to bfintal:

Perhaps we can merge parts of this plugin into core: ​https://wordpress.org/plugins/easy-theme-and-plugin-upgrades/

As stated a year ago, this plugin has existed a long time and works just great. Why not use what's already been tested?

[psykro]

@joyously thanks, I will install that as well and consider it based on the other concerns.

Out of interest sake, what does it take for someone to 'own' a ticket, does it require a chunk of work/discussion to be completed first, or is it just a case of someone making me the owner?

[Clorith]

A ticket owner is really just someone who drives the ticket, you don't need to own it to work on it.

[psykro]

Replying to Clorith:

A ticket owner is really just someone who drives the ticket, you don't need to own it to work on it.

Gotcha, thanks @Clorith.

[psykro]

Right, first attempt at suggesting a possible way forward. To my mind, the simplest way would be to check the zip being installed against any existing possible matches, before the actual file is uploaded.

So on adding a new plugin and clicking the Install Now button

1. Make a WP Rest API call to check if a plugin with either a full or partial slug match exists
2. Display the 'This plugin you are attempting to install seems to exist' message to the user
3. Display the matching plugin details (name, description and version number)
4. Move the Install Now button under the 'confirmation' message and change it to 'Accept and install'
5. On clicking the install button the usual upload and install process proceeds

Things to consider

1. During the upgrade process, the plugin being overridden should probably be renamed/moved to a temporary location, in case the plugin install fails and a rollback is required (sometime like rename to _plugin_name
2. Once the new plugin has successfully been installed, clean up the temporary location at step 1 above.

[psykro]

Replying to joyously:

Replying to bfintal:

Perhaps we can merge parts of this plugin into core: ​https://wordpress.org/plugins/easy-theme-and-plugin-upgrades/

As stated a year ago, this plugin has existed a long time and works just great. Why not use what's already been tested?

The plugin does handle the uploading of a plugin with the same (or similar) plugin slug, by moving and renaming the existing plugin and installing the new plugin.

So I suppose the question is, by adding the pre upload/install notification as outlined above, is that enough of a check against uploading a previous version of the same plugin or uploading a different plugin with the same or similar slug?

[psykro]

Further reading on the the current plugin suggests that it could simply be merged into core as is.

Attention: Version 2.0.0 changed the functionality of the plugin. You are no longer required to select “Yes” from a drop down before the theme or plugin can be upgraded. The need for an upgrade is now detected automatically. So, if you are used to the old functionality of the plugin, do not be concerned about the absence of upgrade details on the theme and plugin upload pages. Simply upload the theme or plugin as if you were installing it, and the plugin will automatically handle upgrading as needed.

However, setting up a real world test, with the same plugin slug but very different contents and the new plugin is uploaded over the old one. So the first plugin step check and verification by the user appears to be required.

[Clorith]

Plugin slug check is definitely required, also remember that the name of a zip file may not be the plugin slug, the zips generally contain a folder with the plugin slug, so an API check before uploading won't be enough here.

[psykro]

Thanks @Clorith

If we uploaded the file asynchronously, using the same javascript library as the media library (which uses plupload as far as I know) this would give us the ability to perform the API check on the plugin slug.

So to update my proposal above, on clicking 'Install Now'

1. Upload plugin zip asynchronously to a temporary location in the uploads directory, showing am 'upload' progress meter
2. Once uploaded, unzip the plugin to use the plugin slug (folder name) to make a WP Rest API call to check if a plugin with either a full or partial slug match exists
3. Display the 'This plugin you are attempting to install seems to exist' message to the user
4. Display the matching plugin details (name, description and version number)
5. Move the Install Now button under the 'confirmation' message and change it to 'Accept and install'
6. On clicking the Accept and install button the plugin is installed

Things to consider remains the same.

[westonruter]

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

[johnbillion]

This is making good progress, but it needs to go into a release cycle earlier on for testing purposes. Punting to 5.0.

[imath]

Hi,

As i'm currently working on this feature for one of my plugin, i'm suggesting this alternative approach. I agree it would be interesting to upload / unzip / explore the uploaded plugin into another temporary folder then ask the user if he wants to update a matching existing plugin with the uploaded one, delete the old plugin, move the temporary folder etc... It would probably allow to check if the version is upper etc..

On the other hand, I think it's a lot of file manipulation and probably edits into the Plugin Upgrader class. The alternative approach is to consider a replace process instead of an update process. There can be cases when you updated a plugin, something's going wrong with your site's configuration and you need to come back to a previous version of the updated plugin to fix the issue with your site.

In 9757.1.patch, i'm using JavaScript to get the Zip filename and use this as the plugin's slug to check for existing plugin thanks to a Rest API request. If there is no match, it doesn't change anything to the process, if there's one, then it adds some information before the "Install Now" button and a "Cancel" button after it to reset the form, see screenshot beneath.

When the user clicks on "Install Now" the process is very similar to an update, and there is very few edits to Plugin_Upgrader::upgrade(). As the process is a bit different, strings need to be a bit different in this case though, because we don't know if the user is upgrading or downgrading the plugin. That's why the patch is taking this in account as you can see beneath :

"Replacing the installed version" & "Plugin replaced successfully" are the new strings.

NB: the patch is using the wp.apiRequest (introduced in 4.9, btw thanks a lot and congrats to people who worked on it 👍) And the script where is located this API is not loaded on multisite configs. There's probably a good reason for this but at the same time as some widgets are using it, i guess they could fail on multisite configs..)

If you think it's an interesting approach, I can give a hand during 5.0 dev cycle to carry on improving the patch and adding Themes support. Else, i'll be happy to give my feedbacks on @psykro 's approach.

[ahortin]

@imath Using the zip filename as the slug is not a great idea. A lot of 'premium' plugins typically have the version number as part of the zip filename. As an example, two very popular plugins Elementor Pro and Gravity Forms, have the following filenames for their most recent versions, elementor-pro-1.9.3.zip and gravityforms_2.2.5.8.zip, respectively.

Using these filenames as the slug would mean that when you checked for an existing version, it would be looking for the wrong slug as the actual folder names within each of those zip files I mentioned, are elementor-pro and gravityforms.

[imath]

@ahortin

Thanks for your feedback, yes i know WordPress & GitHub are also doing so. I trust site admins and i think they are smart people :)

• They can rename a package,
• Plugin Authors can add a line in their description/documentation "If you want to update manually rename the package to elementor-pro.zip"
• The help tab of the administration screen can also include a section to explain how to rename a zip file containing the plugin version.

We could also sanitize the filename from the JavaScript file. But if you think it's not a great experience and it's best to do all the file manipulations on the server, i'm fine with it and i totally understand.

[Clorith]

We can't expect users to rename their files to do an update, that's a bad experience for the average user just looking to update their theme or plugin like they were told to do.

Your concern for reverting a plugin if an update is bad is legitimate though, we could possibly replicate the new code editor behavior of making a call to the site and checking if it fails, at least you won't end up with a white screened site then?

[imath]

@Clorith

Ok then we could use the FileReader API with the external ​JSZip library as i'm suggesting in 9757.2.patch. This way the real file structure contained into the zip package could be checked, no matter the name of the zip file. In the following test I've renammed preferred-languages.zip to RandomName.zip :

[ahortin]

I don't think renaming the zip file is a very good user experience. If someone selects preferred-languages.zip to upload, and then you go and rename this to RandomName.zip, it's going to cause confusion. The user isn't going to be expecting their filename to get changed and if they see a different filename being displayed and uploaded, from the one they selected, it's going to cause them concern.

[imath]

@ahortin

It was an example to show that the real zip content was taking in account so of course it works with preferred-languages-1.2.4.zip etc... The patch doesn’t rename the file at all. Feel free to test it btw :)

Sorry i caused a confusion here.

[bloggingthemes]

I certainly hope this ticket will keep progressing. One of the biggest user experience one could hope for is to give them the method of updating an existing theme and/or plugin simply by re-uploading using the installer. Most users are scared of FTP (I don't blame them), but using a third party plugin is not ideal because who knows how long it will be kept up-to-date and supported.

This is definitely important to exist within the core and should have been there since the beginning. I get so many users say to me:

• How come I can't update my theme using the installer?
• How come I can update with 1-click themes and plugins I get from wordpress but not yours?
• Or they use the installer and get that "already exists" message and get frustrated.

Overall, I hope this gains ground for an upcoming WordPress release because people need this.

I've been using Joomla for 10 years and they've always had this capability. It also doesn't matter what the zip file name is either because it looks at the file's xml info. In the case of WordPress, I would assume this would be a similar equivelent of the text-domain and version number of the theme or plugin. Ultimately, it shouldn't matter if the theme or plugin was installed from .org or elsewhere, the fact is that for a much better user experience, we need this.

Hopefully this makes it in an upcoming release.

[pento]

#46223 was marked as a duplicate.

[galbaras]

From what I can see, there are at least 110,000 active installs of plugins overriding the install function in the quick and dirty manner. The above proposal will give them a better solution.

Also, why not leave the install alone and ADD a function to upload a file for the purpose of updating the plugin/theme? Then, WordPress can assume this is an update and act accordingly, and there's no confusion anywhere.

There can be a new page for selecting a plugin to update and a file to upload, but the easiest way would be to add an "update from file" link on the Installed Plugins page, making it clear in advance what's being updated.

<!--
Hi there! Thanks for contributing to WordPress!

Pull Requests in this GitHub repository must be linked to a ticket in the WordPress Core Trac instance (​https://core.trac.wordpress.org), and are only used for code review. No pull requests will be merged on GitHub.

See the WordPress Handbook page on using PRs for Code Review more information: ​https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/

If this is your first time contributing, you may also find reviewing these guides first to be helpful:

• FAQs for New Contributors: ​https://make.wordpress.org/core/handbook/tutorials/faq-for-new-contributors/
• Contributing with Code Guide: ​https://make.wordpress.org/core/handbook/contribute/
• WordPress Coding Standards: ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/
• Inline Documentation Standards: ​https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/
• Browser Support Policies: ​https://make.wordpress.org/core/handbook/best-practices/browser-support/
• Proper spelling and grammar related best practices: ​https://make.wordpress.org/core/handbook/best-practices/spelling/

-->

I would like to propose a simple solution to most use cases: allow user to overwrite plugins/themes.

Some points to discussion:

• It's not automatic so is a user decision and probably will be checked after a error from default case.
• I love the "Decisions, not Options", but I guess this is a acceptable decision to be taken by anyone uploading a plugin by zip.
• Maybe add a tooltip description? I'll search a example in admin to keep UX.

With this, we:

• Fix layout for big "Install now" translations (like the Brazilian "Instalar agora").
• Fix layout for mobile help text.
• Accept only zip as plugin upload is not able to handle only-file plugins.

Trac ticket: https://core.trac.wordpress.org/ticket/9757

[mariovalney]

Hey :)

I want to help on this ticket.

By now I would like to propose a simple solution: allow user to overwrite plugins/themes.

Details are in the PR comment above.

Some screenshots:

The overwrite option.

The layout problem.

Btw: too cool the ​new trac feature.

[folletto]

I think that regardless of all the intricacies of the years old discussions there's a "first step" approach we can move forward here, which is allowing an overwrite with explicit confirmation.

In short:

1. If the theme/plugin doesn't exist, just do as it does today
2. If the theme/plugin does exist, show a screen asking "Are you sure you want to overwrite?"

There are some complexities in the second case (reading the zip, cleanup of folders, etc) for sure. From a UX perspective, the ideal scenario is to ask, and show at least the version number, name, and general metadata of both so the user can take a decision.

[mariovalney]

I agree.

Actually my first thinking was to add a "override anyway" after the error message in the uploading page. But it was too complicated to a first purpose without discussion haha

If we agree on follow this path I'll would be happy to refact the patch/PR.

About UI, I guess we can use the output we already have, just adding a link to allow the overwrite and a simple table with the comparison you suggested (at least to this first see).

[folletto]

If you've already an idea just post the screenshots of the new flow and we can move from there :)

[mariovalney]

It's just a "console editing" version. So I'm not sure about implementation, but I guess it's possible.

[pingram3541]

Replying to mariovalney:

It's just a "console editing" version. So I'm not sure about implementation, but I guess it's possible.

This is great. I'd just like to add that this would be convenient if it also allowed older versions to "Override anyway" making a roll-back just as easy.

[folletto]

That seems to me a good way forward by keeping it very grounded for a ticket this old that is easily sidetracked. :)

I would just suggest a slight change of language for the actions:

• Red → "Overwrite with uploaded" on the right
• Blue → "Cancel and go back" on the left

I'm suggesting to flip order so the two actions are order-wise associated with the columns right above, which makes it easier to understand: left column and left actions are both "current", while right column and right action are both "uploaded".

Also similarly added the same word inside the red button, so the label of the column matches the label on the button, and for the blue one it makes clear nothing happens ("cancel") and the outcome ("go back").

Also agreed to allow downgrades too.
It should show it in ANY override case (i.e. every time a folder with the same name exists).

That's a good direction!

[pingram3541]

+1 on that, makes sense that the order of presentation matches and the instruction is very clear.

[mariovalney]

Replying to pingram3541:

This is great. I'd just like to add that this would be convenient if it also allowed older versions to "Override anyway" making a roll-back just as easy.

I agree. There is no validation about version: that's why I'm using "override".

Great suggestions @folletto
People from design are in another level haha :)

I guess I'm able to finish this approach today. o/

[organizedthemes]

I would advise that you make it work like the Easy Theme and Plugin Updates plugin, which allows us to overwrite the current plugin/theme and then it actually adds the Older version of that plugin/theme as a .zip file in the Media section in WordPress. Smart and works well in case we need to revert. It's a backup.

[mariovalney]

I like your suggestion but I guess we can keep things simple by now.

Besides that user already have all the information to do a right choice. It's like add a backup option on deleting plugin.

Maybe we could possibly think about replicate the code editor behavior of making a call to the site and checking if it fails, like Clorith suggested?

[mariovalney]

OK. That was not trivial. hahaha

I would really like a core developer feedback, because I'm not sure implementing this in the "Install Skin" is the best approach. But I was not able to figure another way (at least, without change much more code).

1 - I really tried to keep this for "upload" only.

2 - Would we trigger any action like in upgrade?

I guess don't. Because we are not keeping this for upgrade only. In this case, let's deactivate/activate the plugin?

I added a action on overwriting completed to allow developers do whatever they want on "manual update".

3 - I added some CSS. Is that the best place?

4 - As said, we are not checking versions. But we are showing comparison table only if we found plugin data in existing directory and after validating the uploaded package is a plugin.

5 - Maybe implement a new action on update.php to check a new nonce for this package ID?

This will not allow user overwrite a package by adding parameter on URL.

How the table looks like:

Note: themes are very like plugins in this case, so let's do the theme part after eveything is considered about plugins.

[galbaras]

The screenshot looks like the installation is attempted initially and then fails. However, if WordPress now allows overwriting, it's no longer a failure, so perhaps the wording should change there.

Also, since the versions are displayed in a table, why not insert the links into the table for clearer alignment of what is being selected?

Finally, I'd style the links as buttons, because they are actions, not navigations. Button classes can then be "button-primary" (white on blue) for cancelling and "button-secondary" (ghost) for overwriting.

[folletto]

The screenshot looks like the installation is attempted initially and then fails. However, if WordPress now allows overwriting, it's no longer a failure, so perhaps the wording should change there.

I think it's important to show the attempt. I think it's good to show it tried, and found a conflict, and it's asking on how to resolve it.
Granted, I would love for the UI to be less "terminal like", but it's work that has to be done in general for all the upgrades, and shouldn't be in scope for this specific ticket.

Similar answer for the other bits of advice. I know the suggested UI is not perfect, but it's clear and aligned to what we have today. I feel this is especially important for this specific issue as it's 11 years old and got sidetracked multiple times with extensions of scope. If we have a chance to ship this, let's focus.

I'd suggest if you've ideas on how to improve the upgrade screen to open a separate ticket and work on that. :)

[folletto]

I'll leave the dev questions to devs, as for the others:

As said, we are not checking versions. But we are showing comparison table only if we found plugin data in existing directory and after validating the uploaded package is a plugin.

This seems ok to me... I mean, if it's not a plugin and it has no metadata, it shouldn't even try, no?

This will not allow user overwrite a package by adding parameter on URL.

This seems alright in general, but I'm not sure what this means exactly. Can you clarify?

[mariovalney]

Replying to galbaras:

The screenshot looks like the installation is attempted initially and then fails. However, if WordPress now allows overwriting, it's no longer a failure, so perhaps the wording should change there.

That's the idea: we had a failure.
Maybe we can change "more details" to something sounds like "you can fix this way". But I guess a "overwrite this" after a "destination already exists" error do the job.

Replying to folletto:

This seems alright in general, but I'm not sure what this means exactly. Can you clarify?

The "overwrite link" add a parameter to URL. If user add it by hand will do the same without the comparison table, but I guess we can ignore that because the user came from another page and "reload" doesn't work... Rethinking now I was overconcerned hahaha

[joyously]

but I guess we can ignore that because the user came from another page and "reload" doesn't work.

What about with WP-CLI?

[mariovalney]

Replying to joyously:

What about with WP-CLI?

I'm not sure WP-CLI uses this same code, because it runs on "wp-admin/update.php".
But I'm not sure how WP_CLI works too... So I'll take a better look.

Thanks for point this.

[folletto]

Maybe I'm missing something, but WP-CLI seems a different task from the one mentioned here.
Unless there's something simple to do to add support, I'd try to not expand scope further, and save it for later.

[azaozz]

Replying to mariovalney:

The PR is looking good. Some quick notes:

The screenshot looks like the installation is attempted initially and then fails.

That's the idea: we had a failure.

Right, but instead of "failed" it may be better to ask if the user wants to update imho. Then it would go: Installing... The plugin is already installed, do you want to update it? Then the compare table.

I added some CSS. Is that the best place?

Yes, seems okay.

As said, we are not checking versions. But we are showing comparison table only if we found plugin data in existing directory and after validating the uploaded package is a plugin.

Wondering if it should have some sort of warning when "reverting" to an earlier version? Also, it's pretty rare, but there may be plugins with matching names (where one is in the plugins dir and the other is not). Perhaps trying to match more of the plugin meta/headers may be good?

Maybe implement a new action on update.php to check a new nonce for this package ID?

This will not allow user overwrite a package by adding parameter on URL.

May be nice to have but not strictly necessary imho. The plugin-upload nonce is already there so the only way for this to happen would be to manually add the URL query arg. Perhaps can make it a bit more unique, something like add_query_arg( 'overwrite', 'ovewrite-uploaded-plugin', $this->url ).

[galbaras]

Right, but instead of "failed" it may be better to ask if the user wants to update imho. Then it would go: Installing... The plugin is already installed, do you want to update it? Then the compare table.

Totally agree. That's what I meant earlier, in case this wasn't clear.

Wondering if it should have some sort of warning when "reverting" to an earlier version? Also, it's pretty rare, but there may be plugins with matching names (where one is in the plugins dir and the other is not). Perhaps trying to match more of the plugin meta/headers may be good?

There has to be a limit to how much WordPress will do to prevent absolutely every possible issue. Let's assume that if someone hasn't paid attention at first and tried to install a conflicting plugin, they're going to pay attention when the page tells them there's an issue.

What are the chances that a plugin will have both the same name/title AND the same directory name?

As for downgrading, I think that's the main reason for this thread. That, and recovering from an incomplete/corrupt installation.

[mariovalney]

Replying to azaozz:

Right, but instead of "failed" it may be better to ask if the user wants to update imho. Then it would go: Installing... The plugin is already installed, do you want to update it? Then the compare table.

The "folder_exists" and "process_failed" feedback are printed on general upgrader.
It's a lot of changes to achieve that.

I guess it's not a problem an error followed by an retry, so what about changing the UI we can control on specific case (Plugin Install Skin)?

Wondering if it should have some sort of warning when "reverting" to an earlier version? Also, it's pretty rare, but there may be plugins with matching names (where one is in the plugins dir and the other is not). Perhaps trying to match more of the plugin meta/headers may be good?

I was thinking about "overriding" instead "upgrading/downgrading".
It's very plausible after a "folder exists" error... But I agree we should go with the general case (upgrade/downgrade)...

So I improved the feedbacks and the UI.

Now we have a diff alert for all fields:

Except for Version, like upgrade is the secure case:

But we alert about downgrade:

And a text about reuploading (same value for the 5 main fields):

And thanks for the coomments on PR. Already solved :)

@folletto, I've changed the "Overwrite with uploaded" label to match the changing.

And for finish, with this changes we're able to show a better feedback on ovewritting UI:

[folletto]

Sounds good to me. Nicely done.

[mariovalney]

I'm back for themes \o/

A regular update:

And with warnings:

And the same check for "reuploading package" like plugin:

But there's a note about errors on current theme:

And a check for parent theme for uploaded:

---

I removed the "activate" action on already active plugins/themes too.

And there was a cache on theme_info that we need to clear too. No performance problem as it's expected on uploading process.

@azaozz a new code review would be appreciated.

Thanks

[folletto]

Design wise, I think it's good and we can move forward.
Well done.

In reference to earlier comments, I acknowledge this page will need more work at some point in the future (which applies to all updates in general). This is specifically to keep things simple and ship a basic feature that has been missing for 11 years. Once it ships, we can have further discussion and improve it.

[joyously]

This looks great, but I was a little confused by the text "Remove current and install the old version", since in my mind the existing (current) one is the old one, even though the version numbers might say otherwise. I think the text used for all the other cases applies equally to the case of uploading an older version.

[folletto]

I see your point, it could trigger some confusion.

Maybe we can be even more explicit? Instead of "old" we say: "Remove current and install version 1.0.0" (the version number from above), only in the case of an "older" version being uploaded.

Would be difficult to introduce the dynamic version in the "old" scenario?

Otherwise, we can just go back to specify "uploaded", without a special case for "old".
It might work regardless given the highlight in red just above.

[mariovalney]

Done:

Thanks for pointing this.

[folletto]

You're a star. :)

Let's wait for the code review now! :D

[galbaras]

@mariovalney I agree. Very good work.

Can I ask that you make significant text bold? For example "Destination folder already exists" and "The parent theme is missing". The same applies to plugins.

While we're at it, the missing parent theme message can be a lot shorter, while still conveying the same message. No need for the "The current theme has the follow error". It should be enough to say "The parent theme THEME is missing. Please install it first, and then retry".

[Doobeedoo]

Hi, how to not receive email notifications every time someone comments this ticket ?

I tried :

• Trac notifications : everything in unchecked
• My favorites tickets : I do not have any favorite
• Delete my comment on this ticket : I can edit it but not delete it
• Watch this ticket : it's not enabled
• Unsubscribe / do not follow this ticket link in email : no link like this in the email

Many thanks !

[galbaras]

Good question, albeit unrelated. Closest I can find is tweaking https://core.trac.wordpress.org/prefs/notification

[azaozz]

Yes, the workflow/UI is looking good. Perhaps the text strings (copy) can have another look, hoping somebody may have better ideas for there:

• Compare before retry doesn't sound quite right imho. The users can make a decision to upgrade/downgrade at this point. They are not retrying to install again.
• Seems like you are trying to upload a [plugin|theme] already installed.. Something like that could probably be used instead of "Compare before retry"? It comes after "Destination exists...", "Installation failed...", see the screenshot in comment 120.
• Would be good to show "Backup your site before upgrading" warning imho.

Also not sure if this is particularly useful: ​https://github.com/WordPress/wordpress-develop/pull/267/files#diff-84f206c9515d9ad843b821ac8d8d526cR287-R290. As far as I see it shows if there in an error with the currently installed theme. That might be good, but is part of Site Health. Far more useful would be to know if the uploaded plugin/theme has obvious errors.

Speaking of errors, thinking it would be good to check whether the requirements for the plugin/theme are met? Like minimum WP version and PHP version. Discrepancies may get highlighted in the compare table, but seems the upgrade would still be allowed even if not compatible.

Going to ping @dd32 for a look as this patch uses install() to do upgrade, see https://core.trac.wordpress.org/ticket/9757#comment:60. Using install() is pretty much like using FTP to replace the plugin's directory but is missing several steps that are normally performed when upgrading, see https://core.trac.wordpress.org/browser/tags/5.4.1/src/wp-admin/includes/class-plugin-upgrader.php#L169.

TODO:

• [ ] Replace "Compare and retry" with the "same plugin/theme" message.

[galbaras]

Replying to azaozz:

Speaking of errors, thinking it would be good to check whether the requirements for the plugin/theme are met? Like minimum WP version and PHP version. Discrepancies may get highlighted in the compare table, but seems the upgrade would still be allowed even if not compatible.

When downgrading or reinstalling something that's already been active, these checks should not prevent the installation from happening, even if the prerequisites aren't met, although it's good to list the issues on the screen and let the administrator make an informed decision.

[mariovalney]

Thank you for all the points.

Compare before retry doesn't sound quite right imho. The users can make a decision to upgrade/downgrade at this point. They are not retrying to install again.

Retry install (uograde/downgrade) because already failed.

Seems like you are trying to upload a [plugin|theme] already installed.. Something like that could probably be used instead of "Compare before retry"?

Nice. Added a todo.
Ooh... PR comments appears here...

Would be good to show "Backup your site before upgrading" warning imho.

Agreed! We don't show this to regular upload, but as this patches allow users upgrade/downgrade without much validation (because I thinks there's not way to garantee this is a upgrade/downgrade) would be good.

Just like core ones?

---

Important: Before upgrade/downgrade your plugin[theme], please ​back up your database and files.

---

Also not sure if this is particularly useful: ​https://github.com/WordPress/wordpress-develop/pull/267/files#diff-84f206c9515d9ad843b821ac8d8d526cR287-R290.

I was not able to be sure about which part of code are you talking about.

Maybe about the current theme error? I added it because WP treats invalid themes in another way of valid themes (another UI on list theme, for example, and not shows it on "add theme" page). So maybe makes sense to highlight that.

That might be good, but is part of Site Health. Far more useful would be to know if the uploaded plugin/theme has obvious errors.

We can add the check Clorith talked about.

Actually would be great to add this to regular install too. Is it possible? Any direction to point me?

Speaking of errors, thinking it would be good to check whether the requirements for the plugin/theme are met? Like minimum WP version and PHP version. Discrepancies may get highlighted in the compare table, but seems the upgrade would still be allowed even if not compatible.

Good point. Let's add this on "regular install" (check_package method) too?
Thsi way we improve it and will not fall on "ovewrite install" in first place.

[karmatosed]

This seems like a potentially nice adjustment to get into the next release. What is holding up the commit right now and how can we get that happening? I see ui-copy as a keyword, is this just a copy issue now?

[Iamhere]

Hello - came across this thread after find the very useful plugin "​https://wordpress.org/plugins/update-theme-and-plugins-from-zip-file/"

My two cents worth would be

1) Allow us to upload multiple ZIPs at once (e.g. selecting more than just the one zip when clicking "upload from computer" - just queue them up ;-)

2) Remove the blue button after completion that says "activate plugin" as that's confusing (assuming it's already active and that this is just an update :-)

(EDIT re point 2 looks like I didn't see that when scrolling thru the above thread - it seems it's been dealt with.)

Good work team looking forward to this being core!

Best regards

[mariovalney]

Hi!

I reviewed the "Compare before retry" change and I guess "reuploading_plugin" string is not the best approach, as this is a "after-error" procedure. Maybe: "You can check info and retry" ?

Here is the warning (I'm not sure it's the best UI):

And waiting for feedback on:

Let's add this on "regular install" (check_package method) too?
This way we improve it and will not fall on "ovewrite install" in first place.

[mariovalney]

Replying to karmatosed:

This seems like a potentially nice adjustment to get into the next release. What is holding up the commit right now and how can we get that happening?

Hi! I guess we are just waiting for @dd32 feedback (#129) and maybe @azaozz can check we can add the "requirements for the plugin/theme" on regular install too.

I see ui-copy as a keyword, is this just a copy issue now?

I was not able to find reference on handbook... just about keyworks. For curiosity: is there any docs?

P.S.: Sorry the ping, people. Just trying to finish this.

[galbaras]

@mariovalney The screen capture shows "You can check both plugins info and retry". I'd change this to "Please check the information below and choose how to proceed".

Also, I feel that the action links should be more closely related to the table, which refers to "current" and "uploaded". Therefore, I suggest "Keep current version (2.0.0)" and "Install uploaded version (1.0.0)".

Why is the version comparison row highlighted in red? Are you highlighting rows with different values, i.e. if the name is different, that would be highlighted too, or just the version?

Honestly, it's triggering my "fight or flight" response. How about a thin red border instead of the solid background?

[azaozz]

In 9757.3.patch:

• Based on ​https://github.com/WordPress/wordpress-develop/pull/267.diff,
• Tweaked the styling a bit, replaced the red backgrounds with red text (looks more like the rest of wp-admin).
• Updated some strings.
• Fixed few PHPCS errors.
• Moved the "backup now!" warning a bit up and added standard styling.

Thinking this is about ready. Please test! :)

[azaozz]

Uploading - same version.

[azaozz]

Updating to newer version.

[azaozz]

Downgrading.

[azaozz]

I see ui-copy as a keyword

Yeah, this refers to the text that's shown on the screen, including error messages, action links, etc. Still may need some tweaks to make it as best as possible.

Why is the version comparison row highlighted in red?

It's only when downgrading, to add emphasis/draw attention.

Thinking more about it, installing or updating a plugin or theme from a zip is pretty much the same as uploading them through FTP, i.e. the site owner is in full control and can upload anything they want.

However when done through WP it can look at Requires at least and Requires PHP and give more feedback to the user before they decide to go ahead. Perhaps it should check whether the current install (WP version and PHP version) is compatible with the uploaded plugin/theme, and if not, prevent updating? This is already the case when updating from the wp.org API.

To implement this it just needs to look for Requires at least and Requires PHP in the uploaded plugin/theme readme.txt (if not present in the plugin's main file), and do a version_compare() when one or both of these are known. Then show an error and prevent updating if requirements are not met.

Also the Requires at least and Requires PHP text in the UI should probably be replaced with Requires WordPress version and Requires PHP version. Will fix.

[azaozz]

In this case the backup warning should be replaced with "required WP/PHP version not met" error and the "Remove current and install the uploaded version" link should not be outputted?

[azaozz]

In 9757.4.patch:

• Add error messages when the uploaded theme/plugin requirements are not met.
• Show error and prevent updating when the uploaded theme/plugin requires newer WP or PHP version.
• Tweak few function/filter/arg names for (hopefully) better self-documenting.

[azaozz]

Prevent update when required WP version is not met.

[azaozz]

Both WP and PHP "requirement not met" error messages for a plugin.

[azaozz]

Both WP and PHP "requirement not met" error messages for a theme.

[galbaras]

@azaozz Consistency questions: I've never tried to install anything that wasn't compatible with the versions of PHP or WP I had (without overwriting, that is), so I'm not sure what happens in this case. Does WordPress block the installation?

If not, either make WordPress always check (i.e. never trust the admin), or leave these checks out of the overwriting case (i.e. always trust the admin).

[azaozz]

Replying to galbaras:

I've never tried to install anything that wasn't compatible with the versions of PHP or WP I had (without overwriting, that is), so I'm not sure what happens in this case. Does WordPress block the installation?

No, it doesn't block it, but there are several warnings. However when installing through the wp.org API there is quite a bit more info about the theme/plugin. As far as I remember installation is not blocked for "historical reasons", i.e. to support super old plugins that still work despite not being updated for a long time, even before installing plugins from WP Admin was possible. If we were to add this now, I'd think installing would be blocked when something is marked incompatible :)

Can unblock updating by uploading of themes and plugins marked as incompatible if need be, but thinking it's better to keep it. Think WP should respect the minimum requirements set by the theme/plugin authors.

[folletto]

Overall, I agree.

Just for recap, given this is a long thread, I'd remind this is a new feature to enable updating something existing without breaking the site. This is currently not possible, and makes people without FTP access either stuck, or forced to uninstall a theme/plugin before installing it again. This means losing configurations, settings, etc, on a live site, which makes any update experience miserable and makes people less likely to update if they are in this specific situation.

I think that the feature functionality is preserved even with the safety locks included above, even more considering these locks can be worked around fairly easily by editing the themes/plugin. If a plugin declares explicitly a minimum version, would be a mistake to allow it given what we know.

—

As far as the UI design goes: I think it's aligned enough with how this part of WP Admin works, and we can move forward.

I'd encourage further copy edits and refinement to be done once the feature is merged.

[shaunandrews]

theme-errors

[shaunandrews]

plugin-errors

[shaunandrews]

This seems to work as expected. I have some UI requests that I think would help focus and make the screen easier to understand — but I understand I might be asking for too much, and don't want to block to issue from merging.

1. Can we split up the filename from the first heading? Its hard to read right now, and at a glance its not obvious what file I'm uploading.
2. Can we maybe replace the "Plugin/Theme installation failed." line with something a little more descriptive, like "This plugin/theme is already installed."
3. Can we right-align the first column in the table? And maybe adjust the highlighting — no need to highlight _everything_, but just the bits that are important to show the problem.
4. I'm not sure about the use of Notice's for backing up and other issues. My mockups just use normal text.
5. I know this flow currently uses links for the actions, but maybe we could/should switch to buttons?

[poena]

Are these buttons and the highlight with the red background used anywhere else in the admin?

Warnings and errors can not be indicated using color only.

I agree that table can use some more spacing, but I do not think it should prevent the feature from being added.

[folletto]

Warnings and errors can not be indicated using color only.

That's correct in general, but doesn't apply here as they aren't: the color is just highlighting the mismatch in numbers, which is textual and readable. The coloring is just an extra addition.

Also, there's extra copy both in text and on the button action.

[poena]

I understand I was not clear: I meant the red text "This plugin is already installed" and the removal of the admin notice.

The tone that is set is that the first message, in the red bold text, is more important.

[desrosj]

Reviewing this now. Going to stream of thought below:

• "Remove current and install the uploaded version" seems a bit confusing to me. It makes more sense after I re-read the table of information above, but wondering if this can be improved more.
• Should these links be buttons? If not, are these links meeting accessibility?
• The red text for the proceed option is throwing me off. Updating is a good thing, the red makes it feel like the wrong option to click.
• Should there be a call out that any modifications made to the files directly will be erased by proceeding?
• Clicking cancel leaves the zip file in the media library. Should this be cleaned up (probably a follow up discussion)?

I haven't reviewed the code in depth, but it looks good at first glance, and I was unable to find a scenario that broke the process.

[desrosj]

Also, I am in favor of blocking an update when a site does not meet the PHP or WordPress requirements as specified by the plugin or theme. Allowing the update to go through will just risk breaking the user's site, or breaking it due to compatibility issues.

[mariovalney]

Hi. So happy to see this ticket going on.
Let me know if can still help somehow.

About the concern of blocking invalid PHP version, we can add this check to check_package method which validate plugin file.

This will work for both upload and overwrite.

[mariovalney]

Avoid install plugin with wrong PHP requirement.

[azaozz]

In 9757.6.patch: add the changes suggested by @shaunandrews, @poena and @desrosj. Only things I wasn't sure about are the font-size and color for the h2 (This plugin/theme is already installed), and the color/style for the "Replace current with uploaded button. (The colors, font-sizes, button style and placement can be tweaked after commit too.)

Please test as much as possible :) Not just by uploading a zip, also updating core, plugins and themes through the wp.org API, etc. Ideally auto-updates will also be tested, for core, plugins, and themes. Also, feel free to commit :)

we can add this check to check_package method which validate plugin file.

@mariovalney sure, but lets do this from another ticket (after this is committed). This is a bug in the "Install from a zip file" functionality, worth fixing even during beta imho.

[whyisjake]

composer format updates

[whyisjake]

While testing, I am getting three new errors:

1) WP_REST_Plugins_Controller_Test::test_create_item
Undefined property: WP_Ajax_Upgrader_Skin::$overwrite

/build/wp-admin/includes/class-plugin-upgrader.php:89
/build/wp-admin/includes/class-plugin-upgrader.php:125
/build/wp-includes/rest-api/endpoints/class-wp-rest-plugins-controller.php:306
/build/wp-includes/rest-api/class-wp-rest-server.php:1027
/tests/phpunit/includes/spy-rest-server.php:57
/build/wp-includes/rest-api.php:447
/tests/phpunit/tests/rest-api/rest-plugins-controller.php:379

2) WP_REST_Plugins_Controller_Test::test_create_item_and_activate
Undefined property: WP_Ajax_Upgrader_Skin::$overwrite

/build/wp-admin/includes/class-plugin-upgrader.php:89
/build/wp-admin/includes/class-plugin-upgrader.php:125
/build/wp-includes/rest-api/endpoints/class-wp-rest-plugins-controller.php:306
/build/wp-includes/rest-api/class-wp-rest-server.php:1027
/tests/phpunit/includes/spy-rest-server.php:57
/build/wp-includes/rest-api.php:447
/tests/phpunit/tests/rest-api/rest-plugins-controller.php:404

3) WP_REST_Plugins_Controller_Test::test_create_item_and_activate_errors_if_no_permission_to_activate_plugin
Undefined property: WP_Ajax_Upgrader_Skin::$overwrite

/build/wp-admin/includes/class-plugin-upgrader.php:89
/build/wp-admin/includes/class-plugin-upgrader.php:125
/build/wp-includes/rest-api/endpoints/class-wp-rest-plugins-controller.php:306
/build/wp-includes/rest-api/class-wp-rest-server.php:1027
/tests/phpunit/includes/spy-rest-server.php:57
/build/wp-includes/rest-api.php:447
/tests/phpunit/tests/rest-api/rest-plugins-controller.php:431

[poena]

I got this error earlier but I believed it was because I had not applied the patch correctly. -Did you apply the changes made to src/wp-admin/update.php ?

[earnjam]

Fixes unit test errors and PHPCS errors from 9757.6.patch

[earnjam]

Seems to be working well for me in all the various ways I've tested it.

9757.7.patch​ Fixes the unit test errors mentioned by @whyisjake

Trac ticket: https://core.trac.wordpress.org/ticket/9757

[desrosj]

This latest revision looks awesome! Really excited about this.

I ​created and linked a new PR to make reviewing the code easier and to confirm the tests are passing on Travis.

Because the ​`install()` method is used, I don't think that the current implementation would place the site in maintenance mode while the upgrade is applied. If a plugin or theme is active, this could cause some problems.

Placing this before and after the $upgrader->install() call (toggling the boolean) should solve that problem and prevent a user from stumbling on a broken site at during an update.

if ( is_plugin_active() ) {
    $upgrader->maintenance_mode( true );
}

Few more small thoughts, but I don't think any of these are a blocker for committing to include in beta:

• When the PHP or WordPress requirements are not met, can we link to the HelpHub so that the user has some direction: Maybe this page: ​https://wordpress.org/support/update-php/?
• Would it make more sense to create an array of data and ​pass that to this filter before creating the HTML output? If not, I think it makes sense to also ​pass the `$rows` variable to the hook (and maybe this should also be filterable?).

Updated in the latest patch 9757.2.diff​:

• This action hook is a worded a bit weird: upgrader_overwrited_package. The patch replaces that with upgrader_overwrote_package (also toyed with upgrader_overwritten_package). Also, I clarified the docs to state the hook is only fired when the upgrader successfully overwrites the old package.

[SergeyBiryukov]

Noticed a typo in one string in 9757.2.diff​:

• The current theme has the follow error: "%s".

Should probably be "the following".

[shaunandrews]

Whoa, cool. I didn't expect half of my suggestions to become reality. Nice work.

I have two very minor requests:

• In the main heading, can we make "Plugin" and "Theme" lowercase? I could see the argument for capitalizing them, but a quick glance shows me that we don't always do it.
• Similar, the "Name" label in the table (like "Plugin Name" or "Theme Name") should also be lowercase.

[mariovalney]

Replying to azaozz:

But lets do this from another ticket (after this is committed). This is a bug in the "Install from a zip file" functionality, worth fixing even during beta imho.

Perfect! Here is: 50593 :)

[audrasjb]

Thanks for the great work here!

Reviewing the changes, I also noticed a small typo in upgrader_overwrote_package action DocBlock, located in wp-admin/includes/class-theme-upgrader.php:

@param string $package_type The package type (theme or theme).

Should be (plugin or theme). Probably a search and replace issue :)

[azaozz]

Replying to shaunandrews:

I have two very minor requests:

• In the main heading, can we make "Plugin" and "Theme" lowercase? I could see the argument for capitalizing them, but a quick glance shows me that we don't always do it.
• Similar, the "Name" label in the table (like "Plugin Name" or "Theme Name") should also be lowercase.

Sure, the first string is in the (pre-existing) code for installing from a zip. Can change it now though :)

Should be (plugin or theme). Probably a search and replace issue :)

Yep, thanks, fixing :)

[azaozz]

In 48390:

Upgrade/install: Allow plugin and theme updates from a uploaded .zip file.

Props mariovalney, cyberhobo, imath, shaunandrews, mariovalney, earnjam, desrosj, dd32, folletto, swissspidy, melchoyce, pento, joshuawold, psykro, clorith, ahortin, galbaras, pingram3541, joyously, doobeedoo, karmatosed, poena, whyisjake, earnjam, sergeybiryukov, audrasjb, azaozz.

Fixes #9757.

[azaozz]

Replying to desrosj:

Because the ​`install()` method is used, I don't think that the current implementation would place the site in maintenance mode while the upgrade is applied. If a plugin or theme is active, this could cause some problems.

Placing this before and after the $upgrader->install() call (toggling the boolean) should solve that problem and prevent a user from stumbling on a broken site at during an update.

if ( is_plugin_active() ) {
    $upgrader->maintenance_mode( true );
}

Few more small thoughts, but I don't think any of these are a blocker for committing to include in beta:

• When the PHP or WordPress requirements are not met, can we link to the HelpHub so that the user has some direction: Maybe this page: ​https://wordpress.org/support/update-php/?
• Would it make more sense to create an array of data and ​pass that to this filter before creating the HTML output? If not, I think it makes sense to also ​pass the `$rows` variable to the hook (and maybe this should also be filterable?).

Lets handle these in a new ticket.

[afragen]

Just a couple of comments on [48390], there are a couple of non-Yoda conditionals and I think the use of is_wp_version_compatible() and is_php_version_compatible() would make a couple version_compare() conditions simpler.

[joyously]

I tried this in the 5.5beta, and it shows me the HTML tags for the author link.

[joyously]

Having the uploaded zip in the Media Library doesn't really make sense. It isn't mentioned to the user.
If I cancel or click away from the screen, the zip remains in the library. I can't use it from there, though.
If I successfully replace a plugin, neither one is saved in the Media Library. The plugin I've used in the past for this would save the one being replaced, which seems more useful.

[SergeyBiryukov]

In 48447:

I18N: Move the "WordPress Backups" support URL to its own translatable string.

Follow-up to [48390].

See #9757.

[SergeyBiryukov]

Replying to joyously:

I tried this in the 5.5beta, and it shows me the HTML tags for the author link.

Thanks, moved to #50633.

Having the uploaded zip in the Media Library doesn't really make sense. It isn't mentioned to the user.
If I cancel or click away from the screen, the zip remains in the library. I can't use it from there, though.

I think this was addressed in [48417] / #50612.

[SergeyBiryukov]

In 48448:

Upgrade/Install: Use is_php_version_compatible() and is_wp_version_compatible() in plugin and theme requirement checks.

Follow-up to [48390].

Props afragen.
See #9757.

[azaozz]

@SergeyBiryukov [48448] looks good but it changes the conditionals to be "less readable, more inconsistent", see ​https://make.wordpress.org/core/2020/03/20/updating-the-coding-standards-for-modern-php/#comment-38291 :) (that formatting is technically not wrong, the coding standards proposal in that post seems "stuck"..).

Having the uploaded zip in the Media Library doesn't really make sense.

As Sergey mentioned, this is the way installing/upgrading works. Was fixed in [48417], #50612.

Will have to add wp_strip_all_tags() for "Name" and "Author" though.

[azaozz]

In 48453:

Upgrade/Install: Use wp_strip_all_tags() for the fields in the compare table on the "Update theme/plugin from uploaded zip" screen. Some may contain HTML.

See #9757.

[afragen]

Replying to SergeyBiryukov:

In 48448:

Upgrade/Install: Use is_php_version_compatible() and is_wp_version_compatible() in plugin and theme requirement checks.

Follow-up to [48390].

Props afragen.
See #9757.

Both of the compatibility functions include a test for the parameter as empty. It should be sufficient to simply use the compatibility function in the conditional.

If the parameter is empty the condition evaluates to true. It also improves the readability.

[SergeyBiryukov]

Replying to afragen:

Both of the compatibility functions include a test for the parameter as empty. It should be sufficient to simply use the compatibility function in the conditional.

They do, but omitting the check would cause a notice if the plugin data doesn't have the RequiresPHP key:

$new_plugin_data = array();
var_dump( is_php_version_compatible( $new_plugin_data['RequiresPHP'] ) );

Notice: Undefined index: RequiresPHP

It might be that the key always exists at that point, but that doesn't seem to be the case at a glance, so I think including the ! empty() check is a safer option.

[afragen]

Replying to SergeyBiryukov:

Replying to afragen:

Both of the compatibility functions include a test for the parameter as empty. It should be sufficient to simply use the compatibility function in the conditional.

They do, but omitting the check would cause a notice if the plugin data doesn't have the RequiresPHP key:

$new_plugin_data = array();
var_dump( is_php_version_compatible( $new_plugin_data['RequiresPHP'] ) );

Notice: Undefined index: RequiresPHP

It might be that the key always exists at that point, but that doesn't seem to be the case at a glance, so I think including the ! empty() check is a safer option.

In that case, wouldn’t it make more sense to use isset()?

[SergeyBiryukov]

Replying to azaozz:

[48448] looks good but it changes the conditionals to be "less readable, more inconsistent", see ​https://make.wordpress.org/core/2020/03/20/updating-the-coding-standards-for-modern-php/#comment-38291 :) (that formatting is technically not wrong, the coding standards proposal in that post seems "stuck"..).

Personally, I find @jrf's proposal more readable than the alternative, and tend to use the proposed formatting in my commits since the post was published. It's already used in quite a few places in core :)

[azaozz]

Replying to SergeyBiryukov:

Personally, I find @jrf's proposal more readable than the alternative, and tend to use the proposed formatting in my commits since the post was published. It's already used in quite a few places in core :)

I don't have any (strong) preferences, but if it's used for if() then it should also be used for sprintf(), array(), etc. The inconsistency is no good imho :)

[azaozz]

Replying to afragen:

In that case, wouldn’t it make more sense to use isset()?

Guess that's more of a "personal coding habits"? :)

isset() is good in some cases, but if you are expecting a value and doing something with it, it's better to "move on" straight away with ! empty() instead of running more code, calling functions, etc.

[afragen]

Replying to azaozz:

Replying to afragen:

In that case, wouldn’t it make more sense to use isset()?

Guess that's more of a "personal coding habits"? :)

isset() is good in some cases, but if you are expecting a value and doing something with it, it's better to "move on" straight away with ! empty() instead of running more code, calling functions, etc.

In this case the is_*_version_compatible() also does a check for empty().

[SergeyBiryukov]

In 48455:

Upgrade/Install: Simplify compatibility checks for uploaded plugins and themes for better readability.

Use $new_plugin_data and $new_theme_data as a shorthand for the corresponding $this->upgrader properties.

Follow-up to [48390], [48448].

Props afragen.
See #9757.

[SergeyBiryukov]

Replying to azaozz:

Replying to SergeyBiryukov:

Personally, I find @jrf's proposal more readable than the alternative, and tend to use the proposed formatting in my commits since the post was published. It's already used in quite a few places in core :)

I don't have any (strong) preferences, but if it's used for if() then it should also be used for sprintf(), array(), etc. The inconsistency is no good imho :)

To clarify a bit, I don't have a strong preference on if being on its own line, that just seemed more consistent with what's already in core. I do, however, have a strong preference on the boolean/logical operators between the statements being placed at the start of the line, rather than at the end. That's what I find more readable :)

[mariovalney]

​From the tavern maybe we can replace the string:

If you have a plugin in a .zip format, you may install it by uploading it here.

by

If you have a plugin in a .zip format, you may upload it here.

If you have a plugin in a .zip format, you may install or update it by uploading it here.

On install_plugins_upload (plugin-install.php) and install_themes_upload (theme-install.php).

[azaozz]

Replying to mariovalney:

Yeah, makes sense. Thinking the second string is a bit clearer? Fixing.

[azaozz]

In 48509:

Upgrade/install: Fix/clarify the "Upload in a zip format" string.

Props greenshady, mariovalney.
See #9757.

This was merged into core in https://core.trac.wordpress.org/changeset/48390.

[desrosj]

In 48676:

Upgrade/Install: Correct spelling of “overwrite” in new hooks and array indexes.

See #9757.

[desrosj]

In 48677:

Upgrade/Install: Clarify the descriptions for install_(plugin|theme)_overwrite_actions.

See #9757.

[afragen]

There seems to be a difference between https://core.trac.wordpress.org/changeset/48390 and what is currently in core, https://core.trac.wordpress.org/changeset/48390#file4

In install() what is currently in core in $this-run( ... 'clear_destination' => $args['overwrite_package']..).

It should be $parsed_args['overwrite_package'] as $args['overwrite_package'] doesn't exist.

I have no idea how this happened I can't trace the commit.

[sabernhardt]

@afragen I think you're noticing a difference between the plugin and theme upgrader files.

class-plugin-upgrader.php:
'clear_destination' => $parsed_args['overwrite_package'],

class-theme-upgrader.php:
'clear_destination' => $args['overwrite_package'],

For testing, I'll upload a patch to edit the theme upgrader.

[sabernhardt]

changing $args to $parsed_args in class-theme-upgrader.php

[audrasjb]

Dev note published: ​https://make.wordpress.org/core/2020/07/29/miscellaneous-developer-focused-changes-in-wordpress-5-5/

[SergeyBiryukov]

In 48685:

Upgrade/Install: Pass correct argument to clear_destination in Theme_Upgrader::install().

Follow-up to [48390].

Props afragen, sabernhardt.
Fixes #9757.

[SergeyBiryukov]

Reopening for backporting [48685] to the 5.5 branch after a second committer's review.

[desrosj]

Looks good @SergeyBiryukov!

[SergeyBiryukov]

In 48687:

Upgrade/Install: Pass correct argument to clear_destination in Theme_Upgrader::install().

Follow-up to [48390].

Props afragen, sabernhardt.
Reviewed by desrosj, SergeyBiryukov.
Merges [48685] to the 5.5 branch.
Fixes #9757.

[johnbillion]

In 49788:

Upgrade/Install: Remove a duplicate description for the upgrader_overwrote_package action.

See #51800, #9757