Media: Stop forcing crossorigin on IMG tags in media templates - #80532
Conversation
The media templates override added crossorigin="anonymous" to every img tag via str_replace. Under Document-Isolation-Policy: isolate-and-credentialless the browser already loads cross-origin images in credentialless mode, so forcing the attribute triggers a CORS request that breaks previews of images served without CORS headers, such as media offloaded to a CDN. Rework the override to use the HTML Tag Processor: add the attribute only to AUDIO and VIDEO tags that lack it, and strip it from IMG tags where WordPress Core has already added it (7.1 betas), so the plugin also repairs previews on affected Core versions. This mirrors the fix Core applied to wp_add_crossorigin_attributes() in r62048 and avoids the duplicate attributes previously produced on WordPress 7.1. See https://core.trac.wordpress.org/ticket/65673
|
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. |
|
Flaky tests detected in 22bb88c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/29850100249
|
andrewserong
left a comment
There was a problem hiding this comment.
Logic looks consistent with core except for one bit (the removal of crossorigin on img) but that looks intentional.
Code-wise this looks good — I'm going a little faster with reviews today than I'd usually like so I haven't actually tested this PR properly. Just wanted to give a tentative approval in case you're trying to get this in quickly. Happy to take a more detailed look later in the week if you need it, though!
| } elseif ( | ||
| 'IMG' === $tag | ||
| && null !== $template_processor->get_attribute( 'crossorigin' ) | ||
| ) { | ||
| $template_processor->remove_attribute( 'crossorigin' ); | ||
| } |
There was a problem hiding this comment.
Just double-checking: is this to support removing crossorigin that has been added in WP core versions < 7.1? If so that makes sense to me!
There was a problem hiding this comment.
I believe so, let me double check that its really needed.
There was a problem hiding this comment.
Good question - I checked the core history to be sure. No released WordPress version ever adds crossorigin to IMG in the media templates. The injection only existed in pre-releases:
- 7.0 Beta 1-5 carried it via the original client-side media backport, but it was removed in 7.0 Beta 6 (c863860) - 7.0 final has no crossorigin injection at all.
- It came back with the reintroduction on trunk (51a5f4d), so 7.1 Beta 1 and Beta 2 (and nightlies since late May) do add it.
- The core fix (r62819) landed in 7.1 Beta 3, which excludes
IMGagain.
So the removal here only matters for sites still on 7.1 Beta 1/2 (or older nightlies). I think we can just drop it, seems excessive.
There was a problem hiding this comment.
Follow-up: since no released WordPress version ever ships the IMG injection and the Core fix landed in 7.1 Beta 3, the strip only served sites still on Beta 1/2 - not worth the extra code. Dropped it in e330019; the plugin now just leaves IMG tags untouched.
There was a problem hiding this comment.
Follow-up: since no released WordPress version ever ships the IMG injection and the Core fix landed in 7.1 Beta 3, the strip only served sites still on Beta 1/2 - not worth the extra code. Dropped it in e330019; the plugin now just leaves IMG tags untouched.
Looks good 👍
No released WordPress version adds crossorigin to IMG in the media templates, and the core fix (r62819) shipped in 7.1 Beta 3, so the strip only served sites on 7.1 Beta 1/2. Not worth the extra code; the plugin now simply leaves IMG tags untouched.
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
|
I just cherry-picked this PR to the wp/7.1 branch to get it included in the next release: 1c5f420 |
This updates the pinned commit hash of the Gutenberg repository from `4997026b75c922d8a6f77a03d72ed7cad04c7073` to `fd715a6833679d098d9fee84b642f8f1bc27341b`. A full list of changes included in this commit can be found on GitHub: WordPress/gutenberg@4997026...fd715a6 - Update view config API versioning (WordPress/gutenberg#80319) - Perf Tests: Fix 'Selecting blocks' metric reporting 0 ms (WordPress/gutenberg#80524) - Notes: Register the inline note format at import time (WordPress/gutenberg#80576) - Media: Stop forcing crossorigin on IMG tags in media templates (WordPress/gutenberg#80532) - GradientPicker: select by slug so two presets sharing a gradient keep their identity (WordPress/gutenberg#80554) - Media Editor: Show a loading state while the cropped file loads (WordPress/gutenberg#80460) - Remove default paragraph from tab-panel template (WordPress/gutenberg#80565) - Global Styles: Resolve link element styles in block inspector controls for blocks that are links (WordPress/gutenberg#80607) - Media REST API: Backport sideload from url path upload size check (WordPress/gutenberg#80659) - Rich text: remove tabIndex from editable elements again to fix shift+click selection (WordPress/gutenberg#80651) - Gallery: make dynamic mode conversion a single undo level (WordPress/gutenberg#80665) - Background image control: Remove duplicated focus ring (WordPress/gutenberg#80671) - Detach core's note mention kses filter in the baseline strip test (WordPress/gutenberg#80656) - wp-build: sync the page template preload field list with core-data (WordPress/gutenberg#80648) - Read the contentEditable attribute in ownsSelection, not isContentEditable (WordPress/gutenberg#80549) - Writing flow: extend block selections with shift+arrow when there is no native selection (WordPress/gutenberg#80687) - Notes: Capture the target block before saving a block-level note (WordPress/gutenberg#80690) - Theme JSON: Level block-level preset class specificity with :where() (WordPress/gutenberg#80657) - Notes: Sync the sidebar selection to the inline marker under the caret (WordPress/gutenberg#80610) - Writing flow: use isMultiSelecting for shift+click (WordPress/gutenberg#80286) (WordPress/gutenberg#80726) - Block supports: Return from layout support before resolving global settings (WordPress/gutenberg#80771) - Notes: Report save success consistently from note actions (WordPress/gutenberg#80748) - Dynamic Gallery: Rename toolbar button to Detach and add a modal explaining what will happen (WordPress/gutenberg#80727) (WordPress/gutenberg#80774) - ToolsPanel: Migrate styles to an SCSS Module (WordPress/gutenberg#80445) (WordPress/gutenberg#80800) - Add a responsiveEditingEnabled editor setting to hide the Responsive styles option (WordPress/gutenberg#80814) - iOS: remove jumping hack, add typewriter (WordPress/gutenberg#74596) - Writing flow: stop the page scrolling on caret moves within blocks taller than the viewport (WordPress/gutenberg#80708) - Global Styles: Put the inheritance UI behind a Gutenberg experiment (… (WordPress/gutenberg#80818) - Notes: Cancel in-flight hover highlight when focus leaves a note thread (WordPress/gutenberg#80752) - Block Editor: Try to fix typing performance regression (WordPress/gutenberg#80507) - List Block: Preserve ordered type on indent (WordPress/gutenberg#75353) - Make editableRoot a private block setting Symbol, not a public support (WordPress/gutenberg#80820) - Fix cursor position during forward delete of empty blocks (WordPress/gutenberg#80827) - Navigation: Fixes `aria-expanded` not updating on hover submenu inside overlay (WordPress/gutenberg#80828) - Remove redundant @jest-environment jsdom pragma and lint against it (WordPress/gutenberg#80676) - View config: reject shape-mismatched merges, define empty-array semantics, strip nulls from appended members (WordPress/gutenberg#80829) - Editor: leave undo to the browser in fields that handle their own undo (WordPress/gutenberg#80768) - Fix: New route-based admin pages are empty when no JS (WordPress/gutenberg#80839) Props wildworks. See #65529. git-svn-id: https://develop.svn.wordpress.org/trunk@62896 602fd350-edb4-49c9-b593-d223f7449a82
This updates the pinned commit hash of the Gutenberg repository from `4997026b75c922d8a6f77a03d72ed7cad04c7073` to `fd715a6833679d098d9fee84b642f8f1bc27341b`. A full list of changes included in this commit can be found on GitHub: WordPress/gutenberg@4997026...fd715a6 - Update view config API versioning (WordPress/gutenberg#80319) - Perf Tests: Fix 'Selecting blocks' metric reporting 0 ms (WordPress/gutenberg#80524) - Notes: Register the inline note format at import time (WordPress/gutenberg#80576) - Media: Stop forcing crossorigin on IMG tags in media templates (WordPress/gutenberg#80532) - GradientPicker: select by slug so two presets sharing a gradient keep their identity (WordPress/gutenberg#80554) - Media Editor: Show a loading state while the cropped file loads (WordPress/gutenberg#80460) - Remove default paragraph from tab-panel template (WordPress/gutenberg#80565) - Global Styles: Resolve link element styles in block inspector controls for blocks that are links (WordPress/gutenberg#80607) - Media REST API: Backport sideload from url path upload size check (WordPress/gutenberg#80659) - Rich text: remove tabIndex from editable elements again to fix shift+click selection (WordPress/gutenberg#80651) - Gallery: make dynamic mode conversion a single undo level (WordPress/gutenberg#80665) - Background image control: Remove duplicated focus ring (WordPress/gutenberg#80671) - Detach core's note mention kses filter in the baseline strip test (WordPress/gutenberg#80656) - wp-build: sync the page template preload field list with core-data (WordPress/gutenberg#80648) - Read the contentEditable attribute in ownsSelection, not isContentEditable (WordPress/gutenberg#80549) - Writing flow: extend block selections with shift+arrow when there is no native selection (WordPress/gutenberg#80687) - Notes: Capture the target block before saving a block-level note (WordPress/gutenberg#80690) - Theme JSON: Level block-level preset class specificity with :where() (WordPress/gutenberg#80657) - Notes: Sync the sidebar selection to the inline marker under the caret (WordPress/gutenberg#80610) - Writing flow: use isMultiSelecting for shift+click (WordPress/gutenberg#80286) (WordPress/gutenberg#80726) - Block supports: Return from layout support before resolving global settings (WordPress/gutenberg#80771) - Notes: Report save success consistently from note actions (WordPress/gutenberg#80748) - Dynamic Gallery: Rename toolbar button to Detach and add a modal explaining what will happen (WordPress/gutenberg#80727) (WordPress/gutenberg#80774) - ToolsPanel: Migrate styles to an SCSS Module (WordPress/gutenberg#80445) (WordPress/gutenberg#80800) - Add a responsiveEditingEnabled editor setting to hide the Responsive styles option (WordPress/gutenberg#80814) - iOS: remove jumping hack, add typewriter (WordPress/gutenberg#74596) - Writing flow: stop the page scrolling on caret moves within blocks taller than the viewport (WordPress/gutenberg#80708) - Global Styles: Put the inheritance UI behind a Gutenberg experiment (… (WordPress/gutenberg#80818) - Notes: Cancel in-flight hover highlight when focus leaves a note thread (WordPress/gutenberg#80752) - Block Editor: Try to fix typing performance regression (WordPress/gutenberg#80507) - List Block: Preserve ordered type on indent (WordPress/gutenberg#75353) - Make editableRoot a private block setting Symbol, not a public support (WordPress/gutenberg#80820) - Fix cursor position during forward delete of empty blocks (WordPress/gutenberg#80827) - Navigation: Fixes `aria-expanded` not updating on hover submenu inside overlay (WordPress/gutenberg#80828) - Remove redundant @jest-environment jsdom pragma and lint against it (WordPress/gutenberg#80676) - View config: reject shape-mismatched merges, define empty-array semantics, strip nulls from appended members (WordPress/gutenberg#80829) - Editor: leave undo to the browser in fields that handle their own undo (WordPress/gutenberg#80768) - Fix: New route-based admin pages are empty when no JS (WordPress/gutenberg#80839) Props wildworks. See #65529. Built from https://develop.svn.wordpress.org/trunk@62896 git-svn-id: http://core.svn.wordpress.org/trunk@62163 1a063a9b-81f0-0310-95a4-ce76da25c4cd
What?
Fixes #80535
Stops forcing
crossorigin="anonymous"ontoimgtags in the media manager templates.Companion to the Core fix for Trac #65673 (wordpress-develop#12615, wordpress-develop#12616).
Why?
Under
Document-Isolation-Policy: isolate-and-credentiallessthe browser already loads cross-origin images in credentialless mode. Forcingcrossorigin="anonymous"triggers a CORS request that fails for images served withoutAccess-Control-Allow-Originheaders, so media library previews break for offloaded/CDN media.#76618 removed
imgfromgutenberg_add_crossorigin_attributes()and the editor content hook for exactly this reason, but missed the third injection path:gutenberg_override_media_templates()stillstr_replacescrossorigin="anonymous"onto every<img,<audio, and<videoin the Backbone media templates. That staleimgentry was carried into Core by the client-side media processing backport, producing the regression reported in Trac #65673.The plain
str_replacealso produces duplicate attributes on WordPress 7.1, where Core'swp_print_media_templates()now injectscrossoriginitself (<img crossorigin="anonymous" crossorigin="anonymous" …>— visible in the phpunit output before this fix).How?
Reworks the override to mirror Core's Tag Processor approach:
<script type="text/html">template's content and processes it withWP_HTML_Tag_Processor(template text is raw text to the processor, matching Core's implementation inwp_print_media_templates()).crossorigin="anonymous"only toAUDIOandVIDEOtags that don't already have it — no duplicates on WP 7.1, still effective on older WordPress versions.IMGtags untouched. (An earlier revision also stripped the attribute where Core had added it, but no released WordPress version does that — only 7.1 Beta 1/2 did, and the Core fix (r62819) shipped in 7.1 Beta 3 — so the extra code wasn't worth carrying.)The processing logic is extracted into
gutenberg_update_media_template_crossorigin_attributes()so it can be unit tested directly.Testing Instructions
wp_prepare_attachment_for_jsto rewriteurl/sizes[*].urlto an external host, per this gist), on Chrome ≥ 137 over HTTPS or localhost so DIP is active.crossorigin="anonymous"and previews fail with CORS errors in the console. With this PR: nocrossoriginonimgtags, previews load.npm run test:unit:php:base -- --filter media_templatepasses.Automated tests
test_gutenberg_override_media_templatesassertsaudio/videogetcrossoriginand no duplicates are produced.test_gutenberg_update_media_template_crossorigin_attributescovers the add/skip branches and IMG exclusion directly, including non-template<script>content being left untouched.