Introduce filter webp_uploads_pre_replace_additional_image_source to short-circuit replacing additional image sources in frontend content
#319
Conversation
mehulkaklotar
added
[Type] Enhancement
modules/images/webp-uploads/load.php
Outdated
| * @param string $target_mime The target mime in which the image should be created. | ||
| * @param string $context The context where this is function is being used. | ||
| */ | ||
| $image = (string) apply_filters( 'webp_uploads_pre_replace_additional_image_source', $image, $attachment_id, 'full', $target_mime, $context ); |
There was a problem hiding this comment.
This filter should be placed above the line 510 and the code from str_replace would be executed only if the image is different, take a look at suggested approach here:
The full value in this instance should be the name of the size being modified at this point in time.
There was a problem hiding this comment.
A couple of tests would be great as well.
There was a problem hiding this comment.
@mitogh I have added changes as per your feedback in the latest commits.
mehulkaklotar
requested review from
adamsilverstein and
getsource
as code owners
Co-authored-by: Eugene Manuilov <manuilov@google.com>
Co-authored-by: Eugene Manuilov <manuilov@google.com>
When a modern format is available to render the featured image make sure that the replacement image uses the same logic used to replace the images in the content in order to be consistent with the rest of rendered format. Fixes #288
|
Hi there! Can we adjust the webp_uploads_pre_replace_additional_image_source filter code and doc block so it can be documented only one time and for other instances, we can add the below context as we use in WordPress?
Code changes that require for this. Replace this line https://github.com/WordPress/performance/blob/enhancement/replace-additional-image-filter/modules/images/webp-uploads/load.php#L503 code with Replace this whole doc block with https://github.com/WordPress/performance/blob/enhancement/replace-additional-image-filter/modules/images/webp-uploads/load.php#L516-L528 below context. Pull request: #324 Please let me it and let me know your thoughts. |
Co-authored-by: Mukesh Panchal <mukeshpanchal27@gmail.com>
There was a problem hiding this comment.
@mehulkaklotar Production code looks good to me. I have two minor documentation suggestions, more importantly though the test here should be updated to be a more appropriate usage: The filter is for an image HTML tag, so it shouldn't be used to replace the image tag with just a URL.
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
felixarntz
changed the title
webp_uploads_pre_replace_additional_image_source to short-circuit replacing image sources in frontend content
felixarntz
changed the title
webp_uploads_pre_replace_additional_image_source to short-circuit replacing image sources in frontend contentwebp_uploads_pre_replace_additional_image_source to short-circuit replacing additional image sources in frontend content
Summary
Fixes #311
Relevant technical choices
Checklist
[Focus]orInfrastructurelabel.[Type]label.no milestonelabel.