Skip to content

Conversation

@felixarntz
Copy link
Member

@felixarntz felixarntz commented Aug 9, 2023

Summary

Unit tests currently fail in trunk, based on the 6.3 release from yesterday. There are two problems that lead to those failures:

  • WordPress 6.3 makes the Fetchpriority module useless. The module is already not being loaded with that version, so we need to ensure we don't run tests in that case either.
  • WordPress 6.3 actually comes with a breaking change in a low-level function which we use in the WebP Uploads module: Per https://core.trac.wordpress.org/ticket/57685 / https://core.trac.wordpress.org/changeset/55935, editing thumbnails (or all image sizes other than thumbnails) is no longer enabled by default, which is technically a breaking change 😞

This PR brings parity with those changes:

  • Handle the image editing target values thumbnail and nothumb only when editing thumbnails separately is enabled (if using WP 6.3 or greater, per the new filter).
  • Skip tests that aren't relevant based on the related new WP 6.3 functionality.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@felixarntz felixarntz added [Type] Bug An existing feature is broken [Focus] Images no milestone PRs that do not have a defined milestone for release labels Aug 9, 2023
@felixarntz felixarntz changed the title Skip Fetchpriority module tests when on 6.3 or greater Fix WebP handling when editing images based on WordPress 6.3 change Aug 9, 2023
@felixarntz felixarntz requested a review from mitogh as a code owner August 9, 2023 21:31
@felixarntz felixarntz added this to the PL Plugin 2.6.0 milestone Aug 9, 2023
@felixarntz felixarntz removed the no milestone PRs that do not have a defined milestone for release label Aug 9, 2023
@felixarntz
Copy link
Member Author

felixarntz commented Aug 9, 2023

@joemcgill Turns out there is a breaking change in WordPress 6.3 😞

Not sure if it's worth fixing since it's a very low-level function, but in any case I opened https://core.trac.wordpress.org/ticket/59040 for consideration.

Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks @felixarntz for the PR. Left one question.

With the changes we have to update WebP Uploads plugin.

@swissspidy swissspidy mentioned this pull request Aug 10, 2023
3 tasks
@felixarntz
Copy link
Member Author

@mukeshpanchal27

With the changes we have to update WebP Uploads plugin.

Good catch! I've prepared the WebP Uploads standalone plugin for the update in 65b5fdc.

Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks @felixarntz for the changes. LGTM!

@mukeshpanchal27
Copy link
Member

@joemcgill Could you please take a look so we can merge this PR. Thanks!

Copy link
Member

@joemcgill joemcgill left a comment

Choose a reason for hiding this comment

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

The updates look good to me. I've not gotten a chance to review the breaking change in WP yet, but thanks for reporting it.

@mukeshpanchal27 mukeshpanchal27 merged commit 1120a54 into trunk Aug 14, 2023
@mukeshpanchal27 mukeshpanchal27 deleted the fix/broken-63-tests branch August 14, 2023 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature is broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants