WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 3 months ago

#31532 reopened enhancement

Shiny Updates: Don't activate plugins with PHP errors

Reported by: pento Owned by: pento
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: shiny-updates, has-patch, bulk-reopened
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 (18)

#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.


2 years ago

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


2 years ago

#14 @iseulde
5 months ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

This ticket has not seen any activity in over *two* years, so I'm closing it as "wontfix".

The ticket may lack decisiveness, may have become irrelevant, or may not have gathered enough interest.

If you think this ticket does deserve some attention again, feel free to reopen.

For bugs, it would be great if you could provide updated steps to reproduce against the latest version of WordPress (5.0.2 at the time of writing). Remember images or a video can be superior to explain a problem. At the very least, quickly test again to make sure the bug still exists.

If it’s an enhancement or feature, some extra motivation may help.

Thank you for your contributions to WordPress! <3

#15 @JeffPaul
3 months ago

  • Keywords bulk-reopened added
  • Milestone set to Awaiting Review
  • Resolution wontfix deleted
  • Status changed from closed to reopened

A decision was made to reopen tickets that were closed in the bulk edit that this ticket was affected by. This ticket is being placed back into the Awaiting Review milestone so it can be individually evaluated and verified to determine if it is still relevant/valid or reproducible.

Note: See TracTickets for help on using tickets.