Repository navigation
Media: Stop injecting crossorigin attributes under Document-Isolation-Policy - #13446
adamsilverstein wants to merge 9 commits into
Conversation
…-Policy. `isolate-and-credentialless` loads cross-origin subresources without credentials instead of blocking them, so scripts, styles, images, audio, and video from other origins work without a `crossorigin` attribute. Forcing `crossorigin="anonymous"` turns each load into a CORS request, which fails for any host that does not send `Access-Control-Allow-Origin`, such as media offloaded to a CDN. The injection dates from the `require-corp` era of the Gutenberg experiment, where it was needed for anything cross-origin to load. It was carried over unchanged when the editor switched to DIP, and has since caused two regressions (IMG previews, #65673) and a white screen from the tag processor running inside an output buffer display handler (#65930). Send the header directly, deprecate the output buffer helper and the attribute injector, and drop the matching injection from the media templates. Props khokansardar, b0b3k, ianmjones, westonruter, andrewserong. Fixes #65930. See #65673, #64766.
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
Please note the RC for 7.1.1 is September 10th if we want it included in that version. Otherwise it is fine to punt to 7.1.2. |
Bring the branch up to date with the current PHPUnit matrix workflows so a re-run uses the trimmed routine matrix rather than the week-old shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XymJGmRuNKwf8srrtsrcLR
| * Sends the Document-Isolation-Policy header for cross-origin isolation. | ||
| * | ||
| * Uses an output buffer to add crossorigin="anonymous" where needed. | ||
| * `isolate-and-credentialless` loads cross-origin subresources without |
There was a problem hiding this comment.
Starting the sentence with this identifier makes it harder to start reading, IMO.
Maybe start with "The isolate-and-credentialless directive loads…", if that's what this thing is called.
| * @since 7.1.0 | ||
| * @since 7.2.0 | ||
| * | ||
| * @return bool Whether the header was sent. |
There was a problem hiding this comment.
Technically, if headers_sent() returns true then the header() would not get sent either. Should this function return false if headers_sent() returns true?
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Resolve deprecated.php, where trunk appended _truncate_post_slug() at the same spot the branch appended the deprecated cross-origin helpers. Keep both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqepyjmFSryBt88j6f6ubY
header() is a no-op after output starts, so reporting true in that case misleads callers relying on the new return value. Also reword the docblock so it no longer opens with a bare directive identifier. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014d6nhnaeaDimhTNiwB2wwq
andrewserong
left a comment
There was a problem hiding this comment.
This is testing well for me with client-side media processing continuing to work as expected, and PDF embed previews, etc are working in the block editor.
Code-wise this looks like a good companion to WordPress/gutenberg#82614 to me, and the addition of the bool return types helps get the functions here under test (and the test coverage looks thorough).
LGTM! 🚀
Test reportEnvironment — WordPress 7.2-alpha (trunk Reproduced on trunkTwo distinct failures in
With this PRBoth are gone, and all four of my crafted cases pass through unmodified. On a real
The 6 on trunk all land on Underscore.js placeholders inside Automated
Not testedThe original white screen end to end (it needs an error handler that buffers or throws; I did One questionThe ticket is milestoned 7.1.3, a minor release, but this PR also adds a new public function AI assistance: Yes (Claude Code), used for the repro cases, running the suites and drafting this |
Trac ticket: https://core.trac.wordpress.org/ticket/65930
I spent some time testing what
isolate-and-credentiallessactually blocks before deciding how to fix the bookmark bug in #13221, and it turns out thecrossorigininjection is not doing anything useful under DIP. So this removes it rather than patching it. Companion Gutenberg PR: WordPress/gutenberg#82614. Safari keeps its injection through the Client-Side Media Everywhere plugin: adamsilverstein/client-side-media-everywhere#63.Claude ran the browser tests and wrote up the change, here is the summary:
Testing Instructions
Commit message
Claude assembled this message from the PR, the Trac discussion and the props bot:
AI Use
Code and description both written with 🤖 Claude Code. I will review and test.