-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deactivate plugin versions below 4.1 on WordPress 5.0 #11375
Conversation
* Extract rendering logic for LinkContainer into a new presentational component * Give PositionAtSelection its own classname to uncouple it from LinkContainer * Extract link container styles from rich-text into new link container component. Formalise class names. * Add tests for new link container component * Add docs * Provide sane defaults for LinkContainer position and avoid spreading props * Rename component to URLPopover * Export the new component from the editor package * Optionally render the settings button dependent on whether the renderSettings callback is provided * Fix incorrect prop name in docs * Update classnames used in e2e tests * Move responsibility for managing the isEditing state completely to the implementor * Improve docs * Use boolean logic over a ternary * Improve example * Use children instead of RenderURLEditor callback * Add docs for focusOnMount prop
* Use a ref to detect clicks in the autocomplete suggestion list * Add e2e tests to test for regression of link autocomplete issue
* Add list e2e tests * Add list-quote transform tests * Create e2e blocks folder * Add quote transform e2e tests * Add list merging and splitting test
* Ensure multiline prop is either p or li * Make booleans work
* Ensure cite is string when merging quote * Ensure createBlock returns string for html source * Don't merge empty paragraphs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, so this is patching previous versions of released Gutenberg? I guess it's just for releases in the plugin repo and not node packages, etc., right?
The code here seems fine (though I'll point out I haven't tested it myself), but I'm unclear on what's being updated and how.
Conceptually though seems good. 🤷♂️
(If you really need me to test the whole process I could do it, let me know, I'm just being lazy and triaging stuff and I don't quite understand the steps, sorry. 😅) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed the test instructions testing the update on a zip build from this branch and Gutenberg deactivated and the update of WordPress was successful. Using the zip file from 4.0 rc available on GitHub the update of WordPress went wrong and I started getting error 500. So everything went as expected.
* Author: Gutenberg Team | ||
* | ||
* @package gutenberg | ||
*/ | ||
|
||
// Get unmodified $wp_version. | ||
include ABSPATH . WPINC . '/version.php'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can use include once here to be extra safe? But if we are sure including the file multiple types is not problematic or that the file was not included before we can keep the current version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the same reflex about include_once
, but for that particular file include
may be best for two reasons:
version.php
should only contain variable assignments, so no errors from including multiple times should ever happen- by using
include_once
it's theoretically possible that the inclusion gets skipped (due to prior inclusion) and that thewp_version
variable has been overridden elsewhere. Not that I feel too paranoid about this.
Exactly, this is just a small patch for |
Tagged: |
DO NOT MERGE. This is meant to be a patch applied directly to previous releases. Read on.Edit: Patches released tagged. See #11375 (comment)
Ignore the diff against
master
. See actual diff against v4.0.0.Description
Core ticket 45123 took care of systematically deactivating Gutenberg upon upgrading to WordPress 5.0, both as a general precaution and due to the fact that core WordPress becomes the default provider of Gutenberg functionality.
By "general precaution" it is understood that, although core and plugin are expected to be compatible starting with Gutenberg 4.1, they naturally share a large surface and we want to avoid any chance of failure during a core upgrade process.
However, versions of the plugin prior to 4.1 are fundamentally incompatible with WordPress 5.0, to the point where core's procedure of deactivating the plugin happens too late to guarantee isolation, potentially causing a site to WSOD.
Thus, this PR proposes a patch for Gutenberg versions 3.6, 3.7, 3.8, 3.9 and 4.0 that deactivates itself if any version of WordPress above 4.9.8 is detected. It can do this before execution of any other code in the plugin, thus ensuring no Gutenberg code taints core's upgrade routine. With the help of @pento, patch releases will be pushed as auto-updates to existing installations.
The commit list shows an initial version that showed a deactivation notice, but in practice I found this to be useless.
How has this been tested?
Build the plugin from this patch (
bin/build-plugin-zip.sh
).Install and activate it on a WP 4.9.8 site.
Confirm that the plugin stays activated and that the Gutenberg editor is present.
Using the Beta Tester Plugin or any comparable means, upgrade to a WP 5.0 nightly or beta.
Ensure that the process goes smoothly and that it deactivates the Gutenberg plugin.
Repeat the steps using a non-patched version of 4.0 or lower and confirm that the upgrade process is negatively affected by Gutenberg.