WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 6 weeks ago

#31532 assigned enhancement

Shiny Updates: Don't activate plugins with PHP errors

Reported by: pento Owned by: pento
Milestone: Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: shiny-updates has-patch
Focuses: ui, javascript, administration Cc:

Description

Branched from #29820.

When we try to activate a plugin through shiny updates, we shouldn't do it if it causes PHP errors.

Attachments (3)

31532.diff (3.0 KB) - added by pento 4 years ago.
31532.2.diff (4.5 KB) - added by pento 4 years ago.
31532.3.diff (4.9 KB) - added by jorbin 4 years ago.

Download all attachments as: .zip

Change History (16)

#1 @pento
4 years ago

  • Owner set to pento
  • Status changed from new to assigned

#2 @DrewAPicture
4 years ago

  • Keywords needs-patch added

@pento: I take it we'll need a patch here to proceed?

#3 @jorbin
4 years ago

Yes, a patch is needed here. To help get this working and for testing, I've requested a new plugin ( https://wordpress.org/plugins/this-plugin-should-not-be-used/ ) that should be available soon and will contain a syntax error in version 0.2.

#4 @jorbin
4 years ago

https://wordpress.org/plugins/this-plugin-should-not-be-used/ is up. 0.1 should install no problem. 0.2 should break your site.

@pento
4 years ago

#5 @pento
4 years ago

  • Keywords has-patch added; needs-patch removed

31532.diff works, and correctly doesn't activate the broken plugin.

Currently, installing the broken plugin returns the button to the "Install" text, and doesn't show an error. This is... not ideal.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


4 years ago

@pento
4 years ago

#7 @pento
4 years ago

31532.2.diff was a little experiment, but I don't think it's viable. After a plugin has been updated, it does another HTTP request, this time to plugin-install.php. It parses the menu HTML out of it, and sends that back, so we can automatically update the menu.

It doesn't work for a few reasons:

  • It doesn't include any new CSS added, so menus with custom icons end up with a blank icon.
  • The menu hover handler doesn't work once the menu is refreshed.
  • Plugins that do a redirect on activate cause the new menu to have the wrong menu item marked as currently active.

Oh, and there's also the problem of it being dependant on us not changing the wp-admin HTML.

@jorbin
4 years ago

#8 @jorbin
4 years ago

https://cldup.com/uOykf_sdJH-3000x3000.png

attachment:31532.3.diff builds upon attachment:31532.diff from Pento and adds in an error message (both a visual one and .

@helen - I added the customization to div.error to common.css in the notification section, but now I'm wondering if it makes more sense to go in list-tables.css with the rest of the plugin-card code. Any thoughts there?

Thusfar, this is just for the case of installs. We also need to take care of updates and not allowing updates to a version that contains a fatal error. I would love for us to store the current version and restore that version if an update fails, but that may be out of scope for now.

#9 @DrewAPicture
4 years ago

  • Milestone changed from 4.2 to Future Release
  • Type changed from task (blessed) to enhancement

Punting since shiny installs was reverted in [31897], [31900], and [31901].

#10 @geoorgge1968
4 years ago

#32730 was marked as a duplicate.

This ticket was mentioned in Slack in #feature-shinyupdates by swissspidy. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by adampickering. View the logs.


3 years ago

This ticket was mentioned in Slack in #feature-shinyupdates by adampickering. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.