Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#51822 closed defect (bug) (fixed)

Ensure update-core.php knows you're using a development version

Reported by: afragen's profile afragen Owned by: audrasjb's profile audrasjb
Milestone: 5.6 Priority: normal
Severity: normal Version: 5.6
Component: Upgrade/Install Keywords: has-patch has-screenshots commit dev-reviewed
Focuses: Cc:

Description

In r49638 the update-core page was updated and simplified. One of the regressions seems to be that if you are using the Beta Tester plugin and on the Beta/RC Only stream the message will be displayed as

You have the latest version of WordPress.

As you are on a beta/RC it should still say You are using a development version of WordPress.

The attached patch fixes this issue.

Pinging @helen

Attachments (6)

51822.diff (1.1 KB) - added by afragen 3 years ago.
Capture d’écran 2020-11-18 à 19.50.12.png (178.7 KB) - added by audrasjb 3 years ago.
Before patch
Capture d’écran 2020-11-18 à 19.50.29.png (182.3 KB) - added by audrasjb 3 years ago.
After patch
51822.2.diff (1013 bytes) - added by afragen 3 years ago.
remove outdated conditional
51822.3.diff (1.1 KB) - added by afragen 3 years ago.
updated per @azaozz recommendations
51822.4.diff (1.2 KB) - added by SergeyBiryukov 3 years ago.

Download all attachments as: .zip

Change History (29)

@afragen
3 years ago

#1 @afragen
3 years ago

  • Keywords has-screenshots added

#2 @audrasjb
3 years ago

  • Keywords changed from has-patch dev-feedback has-screenshots to has-patch has-screenshots dev-feedback
  • Owner set to audrasjb
  • Status changed from new to accepted

Good point @afragen, this is a regression from the previous changes made in Beta 4.
The proposed patch looks good and fixes the issue (see screenshot above).

@afragen
3 years ago

remove outdated conditional

This ticket was mentioned in Slack in #core-auto-updates by pbiron. View the logs.


3 years ago

#4 @SergeyBiryukov
3 years ago

Thanks for the ticket and the patch!

Just noting that in my testing the results were initially a bit different from the screenshots here.

Without the Beta Tester plugin:

  • On 5.6-RC1: "An updated version of WordPress is available."
  • On 5.6-RC1-49652: "You are using a development version of WordPress."
  • On 5.7-alpha-49651: "You are using a development version of WordPress."

This all actually seems accurate and follows one of the goals of [49638]:

Ensures only one heading between update available, you are on a dev version, and you are on latest appears at any given time, falling back to you are on latest if something strange happens with the returned update data.

So it looks like the regression here is that when there is an update available, you no longer get the "You are using a development version of WordPress" message, but that was the intention of [49638].

However, with the Beta Tester plugin (set to the Beta/RC Only stream as noted) the messages are indeed different and don't mention the development version, because in this case $updates[0]->response appears to be latest rather than development:

  • On 5.6-RC1: "You have the latest version of WordPress."
  • On 5.6-RC1-49652: "You have the latest version of WordPress."
  • On 5.7-alpha-49651: "You have the latest version of WordPress."

This seems like a larger regression.

Without the plugin, 51822.2.diff doesn't seem to make any difference, the messages are still the same, because "An updated version of WordPress is available" message has the higher priority.

With the plugin, 51822.2.diff does fix the issue and displays "You are using a development version" in all three cases.

#5 @afragen
3 years ago

@SergeyBiryukov I absolutely agree with your findings above. I would also like to point out that having the Beta Tester plugin active or using the WP_AUTO_UPDATE_CORE constant set to beta or rc should give similar results.

#6 follow-up: @afragen
3 years ago

The regression is when Beta Tester or the constant is set. If there's an update available it should behave as it does and only report that.

#7 in reply to: ↑ 6 ; follow-up: @azaozz
3 years ago

Replying to afragen:

The regression is when Beta Tester or the constant is set.

Hmmm, should that (also) be fixed in the Beta Tester plugin? Seems Beta Tester is not fully compatible with having WP_AUTO_UPDATE_CORE when set to one of the new values, beta or rc. (As far as I understand these duplicate the same functionality).

Last edited 3 years ago by azaozz (previous) (diff)

#8 @azaozz
3 years ago

Looking at 51822.2.diff, It makes sense to check $wp_version to "see" if it is production or dev. Perhaps the regex would be faster/better as

$development = preg_match( '/alpha|beta|RC/', $wp_version );
Last edited 3 years ago by azaozz (previous) (diff)

#9 in reply to: ↑ 7 @afragen
3 years ago

Replying to azaozz:

Replying to afragen:

The regression is when Beta Tester or the constant is set.

Hmmm, should that (also) be fixed in the Beta Tester plugin? Seems Beta Tester is not fully compatible with having WP_AUTO_UPDATE_CORE when set to one of the new values, beta or rc. (As far as I understand these duplicate the same functionality).

The Beta Tester plugin accounts for a user setting the WP_AUTO_UPDATE_CORE constant to beta or rc. 😉

@afragen
3 years ago

updated per @azaozz recommendations

This ticket was mentioned in Slack in #core-auto-updates by hellofromtonya. View the logs.


3 years ago

#11 @audrasjb
3 years ago

51822.3.diff looks nice to me 👍
It solves the original issue, and addresses the concerns raised by Sergey and Andrew.

#12 @hellofromTonya
3 years ago

@audrasjb Is this ticket ready for commit?

#13 @audrasjb
3 years ago

  • Keywords commit added; dev-feedback removed

I think so. Marking this for commit.

#14 @azaozz
3 years ago

Perhaps I'm overthinking it a bit but to identify a beta or RC "channel", the code currently sets the type of update before making the wp.org API request, and then checks/expects that $updates[0]->response is 'development'. The same is used in core_upgrade_preamble(), core_update_footer(), list_core_update() (twice), wp_get_update_data().

Then 51822.3.diff changes only core_upgrade_preamble() to use $wp_version instead, which makes it a bit inconsistent. What happens if there's an edge case/error with the request and the wp.org API doesn't return 'development'? True, this won't "break" updates, just show (or not show) the appropriate message in the UI. Seems that perhaps the Beta Tester plugin could be extended/enhanced to handle this case better.

Last edited 3 years ago by azaozz (previous) (diff)

#15 @hellofromTonya
3 years ago

  • Keywords dev-feedback added

Adding the dev-feedback keyword so this ticket shows up in the "Commit Candidates which need Dev Review" section on Next Major Release, Workflow Oriented report.

#16 follow-up: @afragen
3 years ago

This issue exists whether you use the Beta Tester plugin or not.

  1. Create a new WP installation and use WP-CLI to update to 5.6-beta1.
  2. Add define('WP_AUTO_UPDATE_CORE', 'beta'); to wp-config.php
  3. Go to update-core.php and update

You will now be at 5.6-RC1. Go to update-core.php you will see the statement You have the latest version of WordPress. This is incorrect and it should state that you are using a development version of WordPress. This is what the patch fixes.

This is a consequence of the changes to the Updates API where if a new update is present in 'development' === $updates[0]->response and other offers are listed as 'latest'.

Fundamentally the old code here in update-core.php doesn't handle the new Updates API responses correctly under certain circumstances. This patch fixes that.

The only other location that could be updated would be in core_update_footer() but that could certainly be in a different ticket/patch. Here, the Beta Tester plugin adds code to return the You are using a development version (%s). Cool! ... text.

I'm not really aware of another issues in those other functions.

The fundamental issue is that the Updates API only shows a 'development' response if an update is available. There is "always" an update available when the channel is on development (Nightlies). If no update is available then the responses are set to 'latest'.

#17 @afragen
3 years ago

#51892 fixes the issue in core_update_footer()

#18 in reply to: ↑ 16 @azaozz
3 years ago

Replying to afragen:

This is a consequence of the changes to the Updates API where if a new update is present in 'development' === $updates[0]->response and other offers are listed as 'latest'.
...
The fundamental issue is that the Updates API only shows a 'development' response if an update is available. There is "always" an update available when the channel is on development (Nightlies). If no update is available then the responses are set to 'latest'.

I see, makes sense, thanks for the explanation.

Another nitpick: There were (still are?) some plugins that "hack" the PHP global $wp_version as a (questionable) security hardening. For that reason few places in core include unmodified $wp_version, see: https://core.trac.wordpress.org/browser/tags/5.5.3/src/wp-includes/update.php#L33. This also bypasses apply_filters( 'bloginfo', $output, $show );.

Thinking perhaps it would be good to do that in core_upgrade_preamble() as otherwise the changed/filtered WP version would result in wrong UI.

#19 @SergeyBiryukov
3 years ago

In 51822.4.diff:

  • Include an unmodified $wp_version, as we already do in a few other places in the admin.
  • Rename $development to $is_development_version, for better readability and consistency with [49708].

#20 @azaozz
3 years ago

  • Keywords dev-reviewed added; dev-feedback removed

51822.4.diff looks good here, +1 to commit to the 5.6 branch too.

#21 @SergeyBiryukov
3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 49709:

Upgrade/Install: Display "You are using a development version" message on WordPress Updates screen for Beta or RC versions.

This ensures that the message is displayed when the WP_AUTO_UPDATE_CORE constant is set to beta or rc and the user is on a development version.

Follow-up to [49245], [49254], [49292], [49638], [49708].

Props afragen, audrasjb, azaozz, SergeyBiryukov.
Fixes #51822.

#22 @SergeyBiryukov
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backporting to the 5.6 branch.

#23 @SergeyBiryukov
3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 49712:

Upgrade/Install: Display "You are using a development version" message on WordPress Updates screen for Beta or RC versions.

This ensures that the message is displayed when the WP_AUTO_UPDATE_CORE constant is set to beta or rc and the user is on a development version.

Follow-up to [49245], [49254], [49292], [49638], [49708].

Props afragen, audrasjb, azaozz, SergeyBiryukov.
Reviewed by azaozz, SergeyBiryukov.
Merges [49709] and [49668] to the 5.6 branch.
Fixes #51822.

Note: See TracTickets for help on using tickets.