FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix WebP handling when editing images based on WordPress 6.3 change by felixarntz · Pull Request #796 · WordPress/performance · GitHub

/ performance Public

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
ConversationCommits (7)ChecksFiles changed
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 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 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 requested changes Aug 9, 2023
View reviewed changes
Comment thread tests/modules/images/fetchpriority/fetchpriority-tests.php Outdated
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 requested a review from mitogh as a code owner August 9, 2023 21:31
felixarntz added this to the PL Plugin 2.6.0 milestone Aug 9, 2023
felixarntz removed the no milestone PRs that do not have a defined milestone for release label Aug 9, 2023

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 requested a review from joemcgill August 9, 2023 21:40
felixarntz mentioned this pull request Aug 9, 2023
Merged
3 tasks
mukeshpanchal27 reviewed Aug 10, 2023
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 reviewed Aug 10, 2023
View reviewed changes
Comment thread modules/images/webp-uploads/image-edit.php Outdated
swissspidy mentioned this pull request Aug 10, 2023
Merged
3 tasks

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 approved these changes Aug 11, 2023
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks @felixarntz for the changes. LGTM!

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 approved these changes Aug 11, 2023
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 merged commit 1120a54 into trunk Aug 14, 2023
mukeshpanchal27 deleted the fix/broken-63-tests branch August 14, 2023 04:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

joemcgill joemcgill approved these changes
swissspidy swissspidy approved these changes
mukeshpanchal27 mukeshpanchal27 approved these changes
westonruter Awaiting requested review from westonruter
pbearne Awaiting requested review from pbearne
adamsilverstein Awaiting requested review from adamsilverstein
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

Back | FazBrowse Home | New Git URL