Skip to content
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

Plugin: Add requirement for minimum and maximum WordPress versions #35194

Open
wants to merge 17 commits into
base: trunk
Choose a base branch
from

Conversation

azaozz
Copy link
Contributor

@azaozz azaozz commented Sep 28, 2021

Description

Limits loading of Gutenberg when the WordPress version is too old or too new. This will prevent errors and incompatibilities in the future. When the plugin is not loaded it asks the user to either turn plugin auto-updates on, update the plugin, or upgrade WordPress (if they can).

When running a newer development version of WP Gutenberg is loaded and adds a warning that it needs updating and will stop working after WordPress is upgraded to the next release version.

  • Adds GUTENBERG_MIN_WP_VERSION and GUTENBERG_MAX_WP_VERSION constants.
  • Adds couple more warnings when Gutenberg is too old or too new for the WP installation.
  • Adds warnings in the plugins list table row when the plugin is not loaded.

To test: override $wp_version in wp-includes/version.php (better) or change the requirements in gutenberg_pre_init().

(The error/warning strings can probably be enhanced. Edit as you see fit.)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

Copy link
Contributor

@Nikschavan Nikschavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed some minor typos

gutenberg.php Outdated Show resolved Hide resolved
gutenberg.php Outdated Show resolved Hide resolved
gutenberg.php Outdated Show resolved Hide resolved
@stevenlinx stevenlinx added the [Type] Experimental Experimental feature or API. label Oct 29, 2021
@noisysocks noisysocks added Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts and removed [Type] Experimental Experimental feature or API. labels Nov 10, 2021
gutenberg.php Outdated
gutenberg_pre_init();
// Minimum supported version. Has to match "Requires at least" header above.
if ( ! defined( 'GUTENBERG_MIN_WP_VERSION' ) ) {
define( 'GUTENBERG_MIN_WP_VERSION', '5.7' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could auto-generate this constant. This would be similar to how GUTENBERG_VERSION gets injected when building the plugin zip:

if (
! $plugin_version &&
preg_match( '@^\s*\*\s*Version:\s*([0-9.]+)@', $line, $matches )
) {
$plugin_version = $matches[1];
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, has to match the Requires at least from the plugin's headers (top of this file). No need to type it twice :)

gutenberg.php Outdated
// Also note that `version_compare()` considers 1 < 1.0 < 1.0.0 (before suffixes)
// so 6.0 < 6.0.0-alpha and 6.0-beta < 6.0.0-alpha are both true. For that reason
// the constant should be set to major (two digits) WordPress version.
define( 'GUTENBERG_NEXT_UNSUPPORTED_WP_VERSION', '6.0' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be risky, but maybe we could also auto-generate this constant from Tested up to:

Tested up to: 6.3

Copy link
Contributor Author

@azaozz azaozz Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, was thinking the same. Generally the Tested up to is set to the currently released WP version, will need to increment it.

Was wondering, what would be a good rule-of-thumb max WP version? As far as I see fixes and new features that get added to Gutenberg while WP is in alpha are generally synced (merged) with core trunk before WP is released, i.e. in the same WP version. The question is: should it stop loading Gutenberg if WP is newer than the Tested up to +1, or should it just show a warning to update Gutenberg in Tested up to +2, and stop loading in Tested up to +3?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be risky, but maybe...

Looking a bit more, actually not sure this would work well. The problem is that if Gutenberg is released during WP alpha or (early?) beta, this should be set to Tested up to +1. However if Gutenberg is released during WP RC, this should probably be set to Tested up to +2 as any new features or bugfixes will most likely not be added to the WP RC branch (branching of WP trunk usually happens shortly after RC1). So seems this would have to be set manually. Of course there will be a lengthily explanation in inline comments about how to do it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it's very difficult to reason how that could work. I need to think a bit more about all the relations between the Gutenberg versions backported to the given major version of WordPress core, Tested up to, the release process, etc. The goal from my perspective would be to automate as much as possible so people don't have to go through the same painful process again reasoning about all the implications 😄

Copy link
Contributor Author

@azaozz azaozz Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

people don't have to go through the same painful process again reasoning about all the implications

Yea, automating the decision would be best. Not sure if that would be possible though, seems too many variables and possible exceptions. The next best thing would be to add as much help and explanations as possible. Thinking it would be great to have "couple pages long" docblock there, in any case.

@gziolo
Copy link
Member

gziolo commented Oct 10, 2023

It would be a very useful addition for sites using the Gutenberg plugin. In particular, it would help avoid the situation when the plugin downgrades the version of the editor that is shipped in WordPress core.

@github-actions
Copy link

github-actions bot commented Oct 12, 2023

Flaky tests detected in ddc4ce3.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6974358369
📝 Reported issues:

gutenberg.php Outdated
/**
* Add a "Gutenberg version is too old" plugins list table notice.
*
* @since 11.7.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has been open for quite some time. These number will need to be updated before landing 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, still going through it at the moment. Should have a better/cleaned up version by tonight.

@gziolo gziolo added the [Type] Enhancement A suggestion for improvement. label Oct 13, 2023
@gziolo gziolo changed the title Add requirement for minimum and maximum WordPress versions Plugin: Add requirement for minimum and maximum WordPress versions Oct 13, 2023
@azaozz
Copy link
Contributor Author

azaozz commented Oct 13, 2023

Warnings and error messages (suggestions for improving the text welcome):
Screenshot 2023-10-13 at 16 19 48
Screenshot 2023-10-13 at 16 20 26
Screenshot 2023-10-13 at 16 22 37
Screenshot 2023-10-13 at 16 34 21
Screenshot 2023-10-13 at 16 34 53

@WPprodigy
Copy link
Contributor

Related trac ticket: https://core.trac.wordpress.org/ticket/59720

gutenberg.php Outdated
} elseif (
version_compare( $wp_version, $max_supported_version, '<' ) &&
version_compare( $wp_version, $max_supported_version . '-alpha', '>=' )
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To test this I set the WP max version to 6.3 and I have 6.4 (beta or RC) installed locally, and I expected the plugin to disable entirely, but for now I just got that it will be disabled when I upgrade. To be honest, I think that's a bit confusing and I'd just disable Gutenberg in this case as well. I'm not sure we need the two levels of feedback.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and made this change. So now:

  • The plugin is also disabled during the beta/RC period. The reasoning is that the Gutenberg features and new code have already been back ported to Core at that time. And you need to update Gutenberg to the next version in order to include the features that were not back ported (targeted for the next WP version).
  • It also means that the max version should be updated right before/after beta 1.

Copy link
Contributor Author

@azaozz azaozz Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also means that the max version should be updated right before/after beta 1

Yep, as soon as the version with the latest fixes and features for the current WP development version is released (the latest Gutenberg version scheduled to be synced with core trunk).

As far as I see further fixes to merged features are done in minor (dot) releases. There GUTENBERG_MAX_WP_VERSION would be same as in the (older) major release.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@mtias
Copy link
Member

mtias commented Oct 26, 2023

The display is a bit odd since it reads like the plugin is active while it's not loaded. Not a blocker, but we could add something like (not loaded) next to the Deactivate action.

@youknowriad
Copy link
Contributor

I tried looking here, I'm not sure there's any filter to that allows us to alter the "name" cell in the plugin table.

@azaozz
Copy link
Contributor Author

azaozz commented Oct 30, 2023

Yea the whole concept of having to install plugins and then activate them is a bit ... different that most other places. Showing "(not loaded)" right next to "Deactivate" sounds good imho. Wondering if "(disabled)" would be even clearer?

Alternatively can probably replace the "Deactivate" with "Uninstall", although technically the plugin is still active. Afaik there's no problem uninstalling active plugins (all "must use" plugins are like that). Will do a bit of testing.

@azaozz
Copy link
Contributor Author

azaozz commented Oct 30, 2023

Added "(disabled)" to the plugin actions links.

disabled-0 disabled-1

@youknowriad
Copy link
Contributor

Looks like there's some failing e2e tests preventing the merge of this PR.

@azaozz
Copy link
Contributor Author

azaozz commented Nov 7, 2023

some failing e2e tests...

Not sure if they are related, or rather cannot figure out what's actually failing. Seems the only possible thing that might fail is when the WP version during the tests is either missing or incorrect/not supported. Cannot see where that is set though.

- Add couple more warnings when Gutenberg is too old or too new for the WP installation.
- Add warnings in the plugins list table row.
azaozz and others added 12 commits November 23, 2023 13:18
Co-authored-by: Nikhil <Nikschavan@users.noreply.github.com>
Co-authored-by: Nikhil <Nikschavan@users.noreply.github.com>
Co-authored-by: Nikhil <Nikschavan@users.noreply.github.com>
Rename GUTENBERG_NEXT_UNSUPPORTED_WP_VERSION to GUTENBERG_MAX_WP_VERSION, etc.
Also tweak the warnings messages a little and simplify the version compare for newer WP versions.
update `@since` to 17.0.0.
@azaozz azaozz force-pushed the fix/maximum-supported-wp-version branch from 27f1cdf to ddc4ce3 Compare November 23, 2023 21:22
Check if e2e tests are failing because of using too new WP version.
@azaozz
Copy link
Contributor Author

azaozz commented Nov 23, 2023

Rebased in an attempt to fix the e2e tests. The timeouts were fixed however "No node found for selector" popped up.

@azaozz
Copy link
Contributor Author

azaozz commented Nov 24, 2023

Seems the e2e tests fail because they are run in an alpha (core trunk) build of WordPress. Tested by increasing GUTENBERG_MAX_WP_VERSION to allow the latest dev WP version, and the e2e tests passed.

Just to confirm: the intention here is to auto-disable (not load) the Gutenberg plugin when it is run in a version of WordPress newer than the "Tested up to" version from the plugin's readme, right? In that case why are the e2e tests being run in a newer version of WP, the current development version? Seems they should be run in the currently released WP version?

@tellthemachines
Copy link
Contributor

why are the e2e tests being run in a newer version of WP, the current development version?

Good question 😄
I think it's to make sure that no new plugin features conflict with new core features, and testing ahead of time that Gutenberg works with the next WP version.

@azaozz
Copy link
Contributor Author

azaozz commented Nov 29, 2023

to make sure that no new plugin features conflict with new core features,

Yea, that's "the other side of the coin". Generally Gutenberg should not be loaded when it will be "downgrading" code that has already been added to core. This may happen more often in the future as "the push" is to move/sync code from Gutenberg to core as soon as possible, while WP is still in alpha.

and testing ahead of time that Gutenberg works with the next WP version

That may not be necessary any more as the current Gutenberg version will not be loaded in the next (released) WP version. Using the GUTENBERG_MAX_WP_VERSION will prevent it.

In any case, changed it back to still load the plugin in the next WP version while it is in development. For example: If GUTENBERG_MAX_WP_VERSION is 6.4, the plugin will load in 6.5 alpha, beta, and RC, but not in the released 6.5.0. That will prevent fatals and other errors in production while still allowing more flexibility for testing.

@tellthemachines
Copy link
Contributor

Generally Gutenberg should not be loaded when it will be "downgrading" code that has already been added to core.

Yeah this is an interesting problem 😄 but given the speed of development in Gutenberg vs core there would only actually be a downgrade if an older version of the plugin were being used. The e2e assume that Gutenberg trunk will always be ahead of core, which I think is correct: trunk should never be behind core. But we also run e2e on release branches when preparing patches for older WP versions, so whatever we go with here should also work in that scenario. I think in the release branches it would make sense to run the tests on whatever WP version they're targeted at.

Copy link

github-actions bot commented Feb 6, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @WPprodigy.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: WPprodigy.

Co-authored-by: azaozz <azaozz@git.wordpress.org>
Co-authored-by: draganescu <andraganescu@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: Nikschavan <nikschavan@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
Co-authored-by: hellofromtonya <hellofromtonya@git.wordpress.org>
Co-authored-by: mtias <matveb@git.wordpress.org>
Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

gutenberg_pre_init();
// Minimum supported version. Has to match "Requires at least" header above.
if ( ! defined( 'GUTENBERG_MIN_WP_VERSION' ) ) {
define( 'GUTENBERG_MIN_WP_VERSION', '6.2' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the min version always known for each Gutenberg release?

What if the plan changes and thus that version might be different (maybe older such as a previous minor or later)? Later isn't necessarily problematic as the remedy is to upgrade. Older is problematic in that a fatal error might happen.

Wondering if instead, this minimum version should be set by Core and consumed in the plugin. For example, Core could provide a global function such as _get_plugin_wp_min_compatible_version() where for-core plugins like Gutenberg could invoke. Let Core determine which version is the min compatible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, sorry but I think I'm missing something? Does that mean we're getting rid of the "Requires at least" plugin header?

What if the plan changes and thus that version might be different

That means a plugin will not "know" what is the minimum WP version it supports at release, and want to change/fix it later? This hasn't been a problem... forever, not sure it is a problem now?

Wondering if instead, this minimum version should be set by Core...
...
Let Core determine which version is the min compatible.

Not sure how core would "know" if a plugin is fully compatible or not. Emphasis on "fully compatible" here as some incompatibilities are not obvious or easy to detect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, this isn't correct. Sorry. It's a minimum version in Core, but a max version in the plugin.

Not sure how core would "know" if a plugin is fully compatible or not. Emphasis on "fully compatible" here as some incompatibilities are not obvious or easy to detect.

Historically within Core itself, it views the minimum compatible version of the GB plugin to be the first version that causes a fatal error. That's very different from defining "fully compatible". It's focused on preventing fatal errors for users.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically the versions that are compatible with the WordPress version in development aren't known until close to WP Beta 1.

Just before Beta 1 or during beta is when the harded code GB version is updated Core (currently within _upgrade_core_deactivate_incompatible_plugins()).

What's my point?

The plugin's actual maximum WordPress version might not be known at the plugin release time. It might change as the WordPress dev cycle advances into beta.

For example, a fatal error happened recently which then caused an update in Core to set the 17.6 as the minimum compat version for WP 6.5. I'm wondering if 17.5.x or older would have know it's max WP version is WP 6.4, and not 6.5?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can released GB plugin versions know if Core itself will deactivate it due to be incompatible?

That's where the plugin could use the proposed Core function _get_plugin_wp_min_compatible_version().

Using the previous example, that Core function would return 17.6. So then released plugin versions could invoke the Core function and take action itself. Then in time once all released versions of the plugin have this new code, Core would no longer need to deactivate the plugin, i.e. because the plugin handles it itself.

Copy link
Contributor Author

@azaozz azaozz Feb 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, this isn't correct. Sorry. It's a minimum version in Core, but a max version in the plugin.

Ahh I see. I knew I'm missing something :)

How can released GB plugin versions know if Core itself will deactivate it due to be incompatible?

The same way it "knows" it is not going to be installed on a server with a PHP version lower than the minimum required, and in a "too old" WP? I.e. the plugin sets its own requirements, and WP makes sure these requirements are met. Same as PHP and WP versions, and now plugin dependencies.

Similar logic has existed in core since 2007 (WP 2.0) I think: plugins can specify minimal WP version. This is opt-in and enforced by the plugins repo API and by the core plugins API.

Adding another optional requirement for a maximum supported WP version would be best to follow the same logic/design imho, and should be available for all plugins. I know of 2-3 other plugins that can use this right now, for example the Test jQuery Updates plugin that will probably be used again for jQuery 4.0.

That's where the plugin could use the proposed Core function _get_plugin_wp_min_compatible_version().

Hmm, don't think WP (core) would be able to "know better" which plugin is compatible with which version. This info should be in the plugin, and is set to a constant in this PR. So not sure why a plugin will have to use a WP function to get the info it sets itself? Think it is the opposite: WP should "read" what the plugin requirements are and check if they are satisfied. If not satisfied WP should not load the plugin and should warn the users/site admins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plugin's actual maximum WordPress version might not be known at the plugin release time. It might change as the WordPress dev cycle advances into beta.

Right. For that reason the "maximum supported" WP version can only be a major (two digits) version and should (generally) match the "tested up to" plugin header.

Minor version of Gutenberg will still support the same "max WP" version. In addition the "max WP" version will include support for the next "WP Trunk", (alpha, beta, RC). This is needed in order to be able to use the current Gutenberg releases in a WP Trunk installs, like on wp.org. Please see the discussion above.

@azaozz
Copy link
Contributor Author

azaozz commented Feb 6, 2024

Just FYI, the reason this PR is not yet merged is that the logic (when to not load newer Gutenberg in older WP) is good, but seems the code would be better off in core, as part of the "PHP fatals prevention" there. We've been thinking/working on that with @hellofromtonya for a while and there will most likely be an initial version of this in WP 6.5.

@WPprodigy
Copy link
Contributor

I really do think it would be good to have the loader-prevention inside this plugin as well though. It's the only place we can fully prevent a bad outcome.

At the end of the day, plugins can be loaded via a require ... statement in PHP, though often there is more sugar on top of that. This being more common in environments that have the plugins managed in version control and activating them is done through code rather than through the UI.

IMO, the main goal here should be to make WP upgrades as safe as possible. And at the moment, having the Gutenberg plugin on a site poses a very big threat as fatals after a WP upgrade are fairly common (due to class name re-use for example).

I haven't closely followed the other work being done in core, but I can't see this fully be solved without the GB plugin preventing itself from loading if it's unsafe to do so. And as far as development lifecycle goes, the only truly required information (as far as I can reason) would be for a core release to determine it's minimum supported version of GB. Because that's when the breakage is occurring.

The sooner we get the loader-prevention code into the GB plugin the better as well, as we won't get the protections right away as not everybody will be using the latest version of the plugin. So starting that timer as soon as possible would be nice.

@azaozz
Copy link
Contributor Author

azaozz commented Feb 17, 2024

I really do think it would be good to have the loader-prevention inside this plugin as well

Well, there's nothing stopping us from adding it in the plugin too, however that would duplicate core's functionality and might eventually interfere with it. The idea here is to add another type of "plugin requirement": maximum WP version. This will be the forth requirement type, the others are: minimum WP version, minimum PHP version, other plugins (as of 6.5).

That way WP will be able to handle these cases in a consistent way which will be easier for the users to follow/understand, and the incompatibility messages/warnings would be a bit better. Also other plugins would be able to set the same requirement.

I can't see this fully be solved without the GB plugin preventing itself from loading

For now the idea is for WP to be more in control when loading plugins. It will need to have the "plugin meta" (the plugin's headers) either easily readable at plugin loading time or caches (in a DB option/persistent cache). Then the plugin will have an "init" function that WP will call if it is safe to initialize the plugin.

This way WP will be able to show all appropriate messages/warnings about incompatible plugins, and prevent loading them when they are updated from FTP or git/svn. Also the warnings can be shown regardless if a plugin is activated or not.

See: https://core.trac.wordpress.org/ticket/60491#comment:13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.