performance icon indicating copy to clipboard operation
performance copied to clipboard

Update WordPress core functions to support multiple mime types

Open mitogh opened this issue 3 years ago • 13 comments

Feature Description

Functions such as:

  • wp_get_attachment_image_src
  • wp_get_attachment_image_srcset
  • wp_get_attachment_image_url

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:

  • #142
  • #149

mitogh avatar Feb 07 '22 16:02 mitogh

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

eclarke1 avatar Feb 08 '22 13:02 eclarke1

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

mitogh avatar Feb 08 '22 16:02 mitogh

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

adamsilverstein avatar Feb 14 '22 16:02 adamsilverstein

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 avatar Mar 04 '22 15:03 jjgrainger

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

jjgrainger avatar Mar 08 '22 15:03 jjgrainger

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 .

mitogh avatar Mar 10 '22 00:03 mitogh

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

mxbclang avatar Mar 10 '22 12:03 mxbclang

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

felixarntz avatar Mar 10 '22 16:03 felixarntz

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:

  • Updating the $icon attribute in core functions to accept a mime type value
  • Updating core functions with an additional $mime_type parameter
  • Creating new core functions to work with attachment mime types

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 avatar Mar 14 '22 12:03 jjgrainger

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.

jjgrainger avatar Mar 22 '22 18:03 jjgrainger

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

mukeshpanchal27 avatar Mar 19 '24 10:03 mukeshpanchal27

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.

joemcgill avatar Apr 01 '24 16:04 joemcgill

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

mukeshpanchal27 avatar Apr 02 '24 06:04 mukeshpanchal27