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

Fix WebP handling when editing images based on WordPress 6.3 change #796

Merged
merged 7 commits into from Aug 14, 2023

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 Issues related to the Images focus area no milestone PRs that do not have a defined milestone for release [Module] fetchpriority Issues for the fetchpriority module 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.

modules/images/webp-uploads/image-edit.php Outdated Show resolved Hide resolved
@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
7 checks passed
@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
[Focus] Images Issues related to the Images focus area [Module] fetchpriority Issues for the fetchpriority module [Type] Bug An existing feature is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants