WordPress / performance

Performance plugin from the WordPress Performance Group, which is a collection of standalone performance modules.
https://wordpress.org/plugins/performance-lab/
GNU General Public License v2.0
340 stars 91 forks source link

Update WordPress core functions to support multiple mime types #155

Open mitogh opened 2 years ago

mitogh commented 2 years ago

Feature Description

Functions such as:

Should be updated in order to support an additional parameter or to update the icon parameter in some cases to behave as mime type support parameter. This parameter would allow specifying the desired mime type for an image in case we want to render the JPEG, WebP, Avif or any other available mime type for an image based on the specified image size.

This is a subsequent step on the work for:

eclarke1 commented 2 years ago

@mitogh assigning this to you if you agree as it relates so closely to #142 and #149 where you're already working

mitogh commented 2 years ago

Sounds good to me I'm assigning @jjgrainger as well to help with the collection of functions

adamsilverstein commented 2 years ago

I'm also going to start looking at this based on the data stored in ~https://github.com/WordPress/performance/pull/154~ https://github.com/WordPress/performance/pull/147

jjgrainger commented 2 years ago

I am working on a proposal as to what changes are to be made. The WIP document is available here and open to early feedback.

jjgrainger commented 2 years ago

The proposal for this is complete and open to feedback. Document is here.

The proposal is to add an additional $mime_type parameter to existing WordPress core functions to request a specific mime type for the attachment.

If the requested mime type is not available, other mime types will be checked for and eventually falling back to the attachments original mime type. A filter to define this order is being discussed in #187

mitogh commented 2 years ago

Thanks @jjgrainger the document looks solid and defines a clear path moving forward.

I'd suggest we can summarize the 3 options and share them during the next call on Tuesday so the rest of the community can provide any concerns/feedback or vote on the different approaches, but let me know what you think about that @adamsilverstein @bethanylang @jjgrainger .

bethanylang commented 2 years ago

@mitogh That makes sense to me, but @adamsilverstein and @felixarntz, please confirm.

@jjgrainger Once you would summarize the options on next week's chat, I can 1) drop a comment for voting here with those summaries and ask folks to leave an emoji for whichever option they'd prefer, 2) add a Needs Decision label, and 3) set a deadline for voting, which is typically two weeks from vote start.

felixarntz commented 2 years ago

@jjgrainger @mitogh @bethanylang I think this topic is a bit too close to code to efficiently discuss in the chat meeting. I'd say we should rather encourage providing more feedback on the doc. Maybe we can just leave a message in the channel anytime today sharing the doc and asking for feedback and preferred approach?

I'm going to have a look today as well. @adamsilverstein Can you please review this doc too? It would be crucial since it closely affects your core patch work.

On that note, I don't think this issue will require any plugin engineering since it's all related to the WP core implementation. That may be obvious, but I certainly wanted to clarify.

jjgrainger commented 2 years ago

Thanks all,

Just wanted to provide an overview of the document shared last week. We are looking to update WordPress core functions to provide a means to retrieve a specific mime type for an attachment.

We're currently exploring 3 options, with a potential fourth that combines some of these. The current 3 within the document are:

These options and others are currently being explored and discussed in the document. This document is open to everyone and welcome to feedback and suggestions on a preferred approach.

If you have anything you want to add please leave all feedback in the document.

I have left a message in the channel also requesting feedback.

jjgrainger commented 2 years ago

I have updated the document with a fourth approach. This approach combines elements of the others outlined and explores creating a new wp_get_attachment_icon() function.

This is open and ready for review/feedback in the document.

mukeshpanchal27 commented 3 months ago

I've come across another support ticket where a user encountered a similar issue: https://wordpress.org/support/topic/how-to-generate-img-secret-including-webp-versions/. The user utilized the wp_get_attachment_image_url and wp_get_attachment_image_srcset functions to retrieve WebP images.

Interestingly, when we disable the Generate JPEG files in addition to WebP option from Settings > Media, these functions still return WebP images, which appears to be a strange behaviour.

cc. @felixarntz @adamsilverstein @joemcgill @eclarke1

joemcgill commented 3 months ago

Thanks @mukeshpanchal27. Are you thinking that this is a bug with the WebP plugin, or with core? If the latter, I think a ticket in Trac would be best.

mukeshpanchal27 commented 3 months ago

@joemcgill, I spent some time checking if it's a core issue or a plugin issue. I also reviewed discussions on other issues/PRs attached to this issue. Since we don't make WebP the default in core, we should fix possible functions that support additional MIME types through action/filters. Something similar to what was in #517.

We can't introduce MIME types in core at the moment, or we can start adding them in separate pieces of work to extend those functions defined here #523. If we agree to make these changes, we can move forward with them.