Modern Image Formats: rewrite img tags from wp_get_attachment_image()#2451
Modern Image Formats: rewrite img tags from wp_get_attachment_image()#2451adamsilverstein wants to merge 8 commits into
Conversation
Until now the Modern Image Formats plugin only rewrote images that flowed through `the_content` (via `wp_content_img_tag`) and featured images (via `post_thumbnail_html`). Any `<img>` built by a direct call to `wp_get_attachment_image()` — page builders, archive loops, custom templates, and many plugins — was left as the original JPEG even when WebP/AVIF sub-sizes were available. Add a new `wp_get_attachment_image` filter that dispatches to the existing rewriter pipeline: `webp_uploads_img_tag_update_mime_type()` in default mode, or `webp_uploads_wrap_image_in_picture()` when picture element output is enabled. URL-returning functions (`wp_get_attachment_image_url()`, `wp_get_attachment_image_src()`, `get_the_post_thumbnail_url()`) are intentionally left untouched, since their return values feed OG tags, RSS, JSON APIs, and other non-HTML consumers where silent format substitution is unsafe. Because `the_post_thumbnail()` routes through `wp_get_attachment_image()`, the dedicated `post_thumbnail_html` registration is now redundant and has been removed; `webp_uploads_update_featured_image()` is marked `@deprecated` but kept in place for any third-party callers. Also make `webp_uploads_wrap_image_in_picture()` idempotent so that markup flowing through both `wp_get_attachment_image` and `wp_content_img_tag` isn't double-wrapped, and extend the context whitelist to include the new `wp_get_attachment_image` context. Fixes WordPress#523.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
…tests Subsequent test fixtures uploading leaves.jpg get renamed to leaves-NN.jpg when leftover files remain on disk, which made the 'leaves.jpg' substring assertion fail in CI. Match on '.jpg' instead — combined with the existing '.webp' negative assertion, the intent (original format preserved) is still verified.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #2451 +/- ##
==========================================
+ Coverage 69.14% 69.21% +0.06%
==========================================
Files 90 90
Lines 7723 7743 +20
==========================================
+ Hits 5340 5359 +19
- Misses 2383 2384 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
adamsilverstein
added
[Type] Enhancement
Featured images are now rewritten through the wp_get_attachment_image filter (via webp_uploads_filter_wp_get_attachment_image()), making this function obsolete. Document the removal as a breaking change in the changelog for third-party callers, who should switch to webp_uploads_img_tag_update_mime_type() or webp_uploads_wrap_image_in_picture() directly, or rely on the new wp_get_attachment_image filter.
|
Took a proper look at this and ran it on a real WooCommerce site (around 500 products, picture mode + AVIF). Good direction, the
Nested Empty One more thing, not a bug - the filter rewrites every I put fixes for all three on a branch, with tests. 148/148 green, and the double-wrap regression test fails without the fix so it actually catches it. Also validated end to end on the demo site. Branch: https://github.com/MarcinDudekDev/performance/tree/fix/2451-review-followups - happy to open it as a PR against this branch if that's easier. AI assistance: Yes |
@MarcinDudekDev Thanks for reviewing and testing. I will bring in your changes, thanks for providing those. |
…ted shim PR WordPress#2451 removed the public function webp_uploads_update_featured_image() outright. It was hooked on `post_thumbnail_html` and is documented `@since 1.0.0`, so deleting it without a deprecation cycle fatal-errors any third-party code that still calls it directly. Restore it in deprecated.php as a thin wrapper that emits a deprecation notice via _deprecated_function() and delegates to the current pipeline (webp_uploads_wrap_image_in_picture() / webp_uploads_img_tag_update_mime_type()). It is no longer registered as a `post_thumbnail_html` filter -- featured images are handled by the new `wp_get_attachment_image` filter -- so this restores backward compatibility without double-processing. readme.txt: move the entry from "Breaking Changes" to "Deprecated". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two robustness fixes to webp_uploads_wrap_image_in_picture().
Nested <picture> (invalid HTML)
------------------------------
When a <picture> produced for a wp_get_attachment_image() call is embedded
in post content, wp_filter_content_tags() extracts the inner <img> and runs
it back through this function via `wp_content_img_tag`. The existing
`stripos( $image, '<picture' )` guard never sees the wrapper, because only
the bare inner <img> substring is passed -- so the image is wrapped twice,
producing nested <picture><picture>...</picture></picture>.
The inner <img> is now tagged with a `data-wp-picture-wrapped` attribute,
and the idempotency guard bails when that marker is present. Context
detection (doing_filter('the_content')) was considered but rejected: it
would skip page-builder images that render during `the_content`. The marker
is documented in the readme changelog.
Empty <picture> wrapper
-----------------------
When no modern-format source resolves for an attachment, the function still
emitted `<picture><img></picture>` with no <source> children. It now returns
the original <img> when $picture_sources is empty.
Adds three tests: a regression test for the in-content double-wrap (embed in
content, run `the_content`, assert exactly one <picture>), an empty-wrapper
test, and a test for the deprecated webp_uploads_update_featured_image() shim.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…get-attachment-image # Conflicts: # plugins/webp-uploads/tests/test-load.php
There was a problem hiding this comment.
Typically we leave this to be update during the release, right? It gets populated automatically.
| false !== stripos( $image, '<picture' ) || | ||
| false !== stripos( $image, 'data-wp-picture-wrapped' ) |
There was a problem hiding this comment.
Is this safe? What if the markup has data-wp-picture-wrapped appearing elsewhere in the string? For example, <img src="foo.jpg" alt="I love the data-wp-picture-wrapped attribute!">. Granted, this is unlikely.
To be more robust, this should use WP_HTML_Tag_Processor to figure out whether it has been previously processed.
There was a problem hiding this comment.
Done in fd98de4. The guard now parses the markup with WP_HTML_Tag_Processor and checks the actual IMG tag's data-wp-picture-wrapped attribute (and bails on a PICTURE tag), so a literal data-wp-picture-wrapped or <picture string appearing inside an attribute value like alt text can no longer cause a false positive.
Co-authored-by: Weston Ruter <westonruter@gmail.com>
…match The idempotency guard in webp_uploads_wrap_image_in_picture() matched the raw `<picture` and `data-wp-picture-wrapped` strings anywhere in the markup, so a literal occurrence inside an attribute value (such as alt text) could falsely flag an image as already wrapped and skip the rewrite. Parse the markup with WP_HTML_Tag_Processor instead and check the actual tag name and the inner img's data-wp-picture-wrapped attribute.
3 participants
Summary
Fixes #523. The
webp-uploadsplugin now rewrites<img>tags produced bywp_get_attachment_image()(and, transitively, bythe_post_thumbnail()/get_the_post_thumbnail()) to serve modern formats (WebP / AVIF) when those sub-sizes are available.Until this change, only images that flowed through
the_content(viawp_content_img_tag) or featured images (viapost_thumbnail_html) were rewritten. Any<img>built directly from a template tag, archive loop, page builder, or custom plugin stayed as its original JPEG. Weston documented the reproduction in #523.Scope note: this intentionally rewrites every
wp_get_attachment_image()caller by default, including output from other plugins. That is a deliberate change of posture from the narrower "don't touch other plugins' output" framing in #2299: serving modern formats everywhere they're available is the goal here. Callers that need the original markup can opt out per-call or wholesale (see below).Changes
webp_uploads_filter_wp_get_attachment_image()inplugins/webp-uploads/hooks.php. Hooks intowp_get_attachment_imageat priority 10, dispatches to the existing rewriter pipeline (webp_uploads_img_tag_update_mime_type()in default mode,webp_uploads_wrap_image_in_picture()in picture-element mode), and guards withwebp_uploads_in_frontend_body()+ a newwebp_uploads_filter_wp_get_attachment_imageopt-out filter.post_thumbnail_htmlregistration.the_post_thumbnail()routes throughwp_get_attachment_image(), so the new filter covers featured images and the dedicated hook became redundant.webp_uploads_update_featured_image()is kept as a deprecated_deprecated_function()shim so third-party callers don't fatal.webp_uploads_wrap_image_in_picture(). Prevents double-wrapping when the same image is processed by more than one rewrite path (e.g. a<picture>fromwp_get_attachment_image()embedded in post content, whose inner<img>is then re-extracted bywp_content_img_tag). The inner<img>carries adata-wp-picture-wrappedmarker, and the guard detects prior wrapping by parsing the markup withWP_HTML_Tag_Processor(checking the actual tag/attribute) rather than matching raw substrings. Empty<picture>wrappers (no resolvable<source>) are also skipped.wp_get_attachment_imagecontext.wp_get_attachment_image_url(),wp_get_attachment_image_src(),get_the_post_thumbnail_url()— URL-returning functions feed OG tags, RSS, JSON APIs, and other non-HTML consumers where silently substituting a modern format is unsafe.Opt-out
Two supported paths:
```php
// Surgical, per-call.
add_filter( 'webp_uploads_filter_wp_get_attachment_image', '__return_false' );
// Wholesale.
remove_filter( 'wp_get_attachment_image', 'webp_uploads_filter_wp_get_attachment_image', 10 );
```
Test plan
webp-uploadstestsuite passes in CI (added ~8 new tests covering default-mode rewrite, picture-mode wrap, idempotency, opt-out filter, `remove_filter` unhook, frontend-body guard, icon placeholder, and double-processing prevention for featured images).