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.

Sign up for GitHub

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

Jump to bottom

Photon: Add webp support #20473

Merged
merged 12 commits into from
Aug 26, 2021
Merged

Photon: Add webp support #20473

merged 12 commits into from
Aug 26, 2021

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Jul 28, 2021

Supports #19654

Changes proposed in this Pull Request:

  • Adds webp support to Photon

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

  • Using WP 5.8, upload a webp image to your site (see test images at https://developers.google.com/speed/webp/gallery1 )
  • Ensure the image CDN feature is active.
  • Before the patch, see that Photon URLs are not used for the webp image.
  • After patch, see that they are.

@kraftbj kraftbj requested a review from a team as a code owner July 28, 2021 20:07
@kraftbj kraftbj added [Focus] Compatibility Ensuring our products play well with third-parties [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Jul 28, 2021
@kraftbj kraftbj self-assigned this Jul 28, 2021
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D64846-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@github-actions github-actions bot added [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ Instant Search [Feature] Search For all things related to Search labels Jul 28, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jul 28, 2021 •
edited
Loading

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

  • Next scheduled release: September 7, 2021.
  • Scheduled code freeze: August 30, 2021.

@kraftbj kraftbj added the [Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins label Jul 28, 2021
@samiff samiff added this to the jetpack/10.0 milestone Jul 28, 2021
@samiff samiff requested review from sdixon194 and samiff July 28, 2021 20:21
samiff
samiff reviewed Jul 29, 2021
View reviewed changes
Copy link
Contributor

@samiff samiff left a comment

Choose a reason for hiding this comment

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

Tested well for me, was able to see working Photon urls in a test gallery and slideshow using webp images.

@kraftbj I did a quick search to see if there was anything else that might need updating, any of these stand out to you?

jetpack/projects/plugins/jetpack/sal/class.json-api-post-base.php

Line 583 in 8cde890

if ( in_array( $ext, array( 'jpg', 'jpeg', 'png', 'gif' ) ) ) {

jetpack/projects/plugins/jetpack/class.json-api-endpoints.php

Line 1370 in 74d0505

if ( in_array( $ext, array( 'jpg', 'jpeg', 'png', 'gif' ) ) ) {

jetpack/projects/plugins/jetpack/_inc/lib/core-api/class.jetpack-core-api-site-endpoints.php

Line 224 in ba2dd6c

get_object_vars( wp_count_attachments( array( 'image/jpeg', 'image/png', 'image/gif', 'image/bmp' ) ) ),

jetpack/projects/plugins/jetpack/functions.global.php

Line 381 in 7e18142

'jetpack_supported_media_sideload_types',

jetpack/projects/plugins/jetpack/modules/videopress/js/videopress-plupload.js

Line 247 in ba2dd6c

image = /(?:jpe?g|png|gif)$/i.exec( file.name );

@kraftbj
Copy link
Contributor Author

kraftbj commented Jul 29, 2021

@samiff I don't think any of those are required for Photon to function, but it does look like they all do to be updated. I would feel more comfortable not including them as part of 10.0, though. With the scope creeping a bit, let's move this to 10.1.

@kraftbj kraftbj modified the milestones: jetpack/10.0, jetpack/10.1 Jul 29, 2021
@kraftbj kraftbj requested a review from samiff August 2, 2021 14:15
sdixon194
sdixon194 previously approved these changes Aug 2, 2021
View reviewed changes
Copy link
Contributor

@sdixon194 sdixon194 left a comment

Choose a reason for hiding this comment

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

This works for me 👍

@sdixon194 sdixon194 added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Aug 2, 2021
@kraftbj kraftbj added [Status] In Progress and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 3, 2021
@sdixon194
Copy link
Contributor

@kraftbj are you still working on this one, or need approval again after the merging with master?

@kraftbj kraftbj added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Aug 25, 2021
@github-actions github-actions bot added the [Feature] VideoPress A feature to help you upload and insert videos on your site. label Aug 25, 2021
@sdixon194 sdixon194 added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Aug 25, 2021
@kraftbj kraftbj merged commit a8a36b8 into master Aug 26, 2021
@kraftbj kraftbj deleted the add/webp-photon-support branch August 26, 2021 19:40
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Aug 26, 2021
@github-actions
Copy link
Contributor

Great news! One last step: head over to your WordPress.com diff, D64846-code, and commit it.
Once you've done so, come back to this PR and add a comment with your changeset ID.

Thank you!

@kraftbj
Copy link
Contributor Author

kraftbj commented Aug 26, 2021

r230845-wpcom

davidlonjon added a commit that referenced this pull request Aug 27, 2021
@davidlonjon
[not verified] Merge branch 'master' into try/boost-javascript-tests-…
206e7e6 
…workflow

* master: (57 commits)
  E2E tests: run against atomic test site (#20807)
  Connection: remove in-place from main connection flow (#20797)
  Restore the 'safecss_embed_style' filter from the pre-4.7 Custom CSS (#20654)
  Photon: Add webp support (#20473)
  Responsive Videos: support more embed block formats (#20834)
  Instant Search: ensure box-shadow and text-shadow aren't applied to search buttons (#20833)
  Search: Apply design polish to the Customberg customization interface (#20754)
  Block editor: add .min suffix to all resources loaded in editor (#20820)
  Release forgotten projects (#20837)
  tools: Update help texts (#20830)
  Remove any wp-env leftovers (#20835)
  CLI: Remove autotagger prompt when generating a new plugin (#20826)
  Enable Support to perform utf8 conversions during checksum calculations (#20816)
  remove unused methods (#20828)
  Remove unnecessary use of `prettier` on JSON data (#20823)
  push-to-mirrors: fix auth for "mirror repo exists" check (#20824)
  cli: Fix tests (#20825)
  Issue templates: update docs & allow selecting no plugin (#20821)
  update annotations versions (#20794)
  Admin: use JetpackFooter RNA component (#20630)
  ...
@jeherve jeherve mentioned this pull request Oct 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anomiex anomiex anomiex left review comments

@sdixon194 sdixon194 sdixon194 approved these changes

@samiff samiff Awaiting requested review from samiff

Assignees

@kraftbj kraftbj

Labels
[Feature] Photon aka "Image CDN". Feature developed in the Image CDN package and shipped in multiple plugins [Feature] Search For all things related to Search [Feature] VideoPress A feature to help you upload and insert videos on your site. [Focus] Compatibility Ensuring our products play well with third-parties [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Milestone
jetpack/10.1
Development

Successfully merging this pull request may close these issues.
6 participants
@kraftbj @matticbot @sdixon194 @anomiex @samiff @bluefuton