Skip to content

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

Merged
mukeshpanchal27 merged 7 commits into
trunkfrom
fix/broken-63-tests
Aug 14, 2023
Merged

Fix WebP handling when editing images based on WordPress 6.3 change#796
mukeshpanchal27 merged 7 commits into
trunkfrom
fix/broken-63-tests

Conversation

@felixarntz

@felixarntz felixarntz commented Aug 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

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
joemcgill
joemcgill requested changes Aug 9, 2023
View reviewed changes
Comment thread tests/modules/images/fetchpriority/fetchpriority-tests.php Outdated
@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

felixarntz commented Aug 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

@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.

@felixarntz felixarntz requested a review from joemcgill August 9, 2023 21:40
@felixarntz felixarntz mentioned this pull request Aug 9, 2023
Merged
3 tasks
mukeshpanchal27
mukeshpanchal27 reviewed Aug 10, 2023
View reviewed changes

@mukeshpanchal27 mukeshpanchal27 left a comment

Copy link
Copy Markdown
Member

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.

Comment thread modules/images/webp-uploads/image-edit.php Outdated
swissspidy
swissspidy reviewed Aug 10, 2023
View reviewed changes
Comment thread modules/images/webp-uploads/image-edit.php Outdated
@swissspidy swissspidy mentioned this pull request Aug 10, 2023
Merged
3 tasks
@felixarntz

felixarntz commented Aug 10, 2023

Copy link
Copy Markdown
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.

mukeshpanchal27
mukeshpanchal27 approved these changes Aug 11, 2023
View reviewed changes

@mukeshpanchal27 mukeshpanchal27 left a comment

Copy link
Copy Markdown
Member

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

mukeshpanchal27 commented Aug 11, 2023

Copy link
Copy Markdown
Member

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

joemcgill
joemcgill approved these changes Aug 11, 2023
View reviewed changes

@joemcgill joemcgill left a comment

Copy link
Copy Markdown
Member

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

Reviewers

@joemcgill joemcgill joemcgill approved these changes

@swissspidy swissspidy swissspidy approved these changes

@mukeshpanchal27 mukeshpanchal27 mukeshpanchal27 approved these changes

@westonruter westonruter Awaiting requested review from westonruter

@pbearne pbearne Awaiting requested review from pbearne

@adamsilverstein adamsilverstein Awaiting requested review from adamsilverstein

@mitogh mitogh Awaiting requested review from mitogh

Assignees

No one assigned

Labels

[Type] Bug An existing feature is broken

Projects

None yet

Milestone

PL Plugin 2.6.0

Development

Successfully merging this pull request may close these issues.

4 participants

@felixarntz @mukeshpanchal27 @joemcgill @swissspidy