Repository navigation
Add crossorigin="anonymous" to IMG elements after Gutenberg core change - #18
Merged
Merged
Conversation
Copilot
AI
changed the title
[WIP] Update images to use crossorigin="anonymous" attribute
Add plugin-owned crossorigin="anonymous" attribute injection
Mar 18, 2026
Owner
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR refactors cross-origin attribute handling by introducing a new public function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
|
@copilot this only needs to target the elements core removed, the remainder are still covered in core. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/Test_Add_Crossorigin_Attributes.php (1)
85-132: Add regression tests for two missing edge cases.Please add coverage for protocol-relative external URLs and for preserving an existing parent
crossoriginwhen processing cross-origin<source>.✅ Suggested tests
+ /** + * Adds crossorigin to protocol-relative cross-origin URLs. + */ + public function test_adds_crossorigin_to_protocol_relative_external_url() { + $html = '<script src="//cdn.example.com/app.js"></script>'; + $result = csme_add_crossorigin_attributes( $html ); + + $this->assertStringContainsString( 'crossorigin="anonymous"', $result ); + } + + /** + * Does not overwrite an existing parent crossorigin when source is cross-origin. + */ + public function test_does_not_overwrite_parent_crossorigin_for_source() { + $html = '<video crossorigin="use-credentials"><source src="https://cdn.example.com/video.mp4"></video>'; + $result = csme_add_crossorigin_attributes( $html ); + + $this->assertStringContainsString( 'crossorigin="use-credentials"', $result ); + $this->assertStringNotContainsString( 'crossorigin="anonymous"', $result ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Test_Add_Crossorigin_Attributes.php` around lines 85 - 132, Add two regression tests to tests/Test_Add_Crossorigin_Attributes.php that exercise csme_add_crossorigin_attributes for (1) protocol-relative external URLs (e.g., an <img> or <source> with src="//cdn.example.com/...") and assert that the parent element gains crossorigin="anonymous", and (2) preserving an existing parent crossorigin when a cross-origin <source> is processed (e.g., a <video crossorigin="use-credentials"><source src="https://..."></video>) and assert the parent's crossorigin remains unchanged. Ensure the new test method names clearly describe the cases (e.g., test_handles_protocol_relative_urls and test_preserves_existing_parent_crossorigin) and use assertions consistent with the file (assertStringContainsString/assertSame/assertStringNotContainsString) to verify behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@includes/cross-origin-isolation.php`:
- Around line 169-174: The SOURCE branch currently finds the parent via
$processor->seek('audio-video-parent') and unconditionally calls
$processor->set_attribute('crossorigin','anonymous'), which can overwrite an
existing parent crossorigin; change this to first inspect the parent’s
crossorigin (e.g. via $processor->get_attribute('crossorigin') or has_attribute
if available on the found $sought) and only call
set_attribute('crossorigin','anonymous') when no crossorigin value is present
(null/empty) so existing values are preserved.
- Line 168: The conditional uses str_starts_with() which breaks PHP 7.4
compatibility; replace calls to str_starts_with($url, $site_url) and
str_starts_with($url, '/') with equivalent strpos checks (e.g., strpos($url,
$site_url) === 0 and strpos($url, '/') === 0) or alternatively raise the
project's minimum PHP version in composer.json/readme.txt to 8.0; update the
condition that references $url, $site_url, and $crossorigin in
includes/cross-origin-isolation.php to use the strpos-based checks to maintain
PHP 7.4 support.
- Line 168: The current condition treating root-relative URLs uses !
str_starts_with($url, '/') which incorrectly classifies protocol-relative URLs
('//cdn...') as root-relative; update the conditional in
includes/cross-origin-isolation.php so the "root-relative" check only matches a
single leading slash (i.e., use str_starts_with($url, '/') && !
str_starts_with($url, '//') when deciding a root-relative URL), so
protocol-relative URLs are considered cross-origin and will get
crossorigin="anonymous" added when appropriate.
---
Nitpick comments:
In `@tests/Test_Add_Crossorigin_Attributes.php`:
- Around line 85-132: Add two regression tests to
tests/Test_Add_Crossorigin_Attributes.php that exercise
csme_add_crossorigin_attributes for (1) protocol-relative external URLs (e.g.,
an <img> or <source> with src="//cdn.example.com/...") and assert that the
parent element gains crossorigin="anonymous", and (2) preserving an existing
parent crossorigin when a cross-origin <source> is processed (e.g., a <video
crossorigin="use-credentials"><source src="https://..."></video>) and assert the
parent's crossorigin remains unchanged. Ensure the new test method names clearly
describe the cases (e.g., test_handles_protocol_relative_urls and
test_preserves_existing_parent_crossorigin) and use assertions consistent with
the file (assertStringContainsString/assertSame/assertStringNotContainsString)
to verify behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8053f88d-1fb9-44d7-a214-6e13e6c8c01a
📒 Files selected for processing (2)
includes/cross-origin-isolation.phptests/Test_Add_Crossorigin_Attributes.php
Contributor
Author
Updated in b1ae169. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Add plugin-owned crossorigin="anonymous" attribute injection
Add crossorigin="anonymous" to IMG elements after Gutenberg core change
Mar 19, 2026
adamsilverstein
marked this pull request as ready for review
March 19, 2026 03:53
Owner
|
@copilot address the feedback from code rabbit |
Contributor
Author
Addressed in f06deee:
The SOURCE → parent crossorigin overwrite issue from CodeRabbit is no longer applicable since the function was narrowed to IMG-only in the previous commit. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Owner
@copilot pretty sure wordpress shims this funciton, so you can undo that change (verify) |
Contributor
Author
Confirmed — WordPress provides a Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This was referenced Jul 16, 2026
Owner
adamsilverstein
requested changes
Jul 16, 2026
adamsilverstein
left a comment
adamsilverstein
left a comment
Owner
There was a problem hiding this comment.
Reviewed against WordPress 7.1 trunk. The premise holds: core's wp_add_crossorigin_attributes() handles AUDIO/LINK/SCRIPT/VIDEO/SOURCE and not IMG, and the delegate-then-augment structure is right. The WP_HTML_Tag_Processor implementation and test coverage look good, including correctly treating protocol-relative URLs as cross-origin (which core gets wrong). A few changes needed:
-
Gate IMG handling on
require-corp(Safari) only.crossorigin="anonymous"switches the image fetch to CORS mode. UnderCross-Origin-Embedder-Policy: credentialless(Firefox), plain no-cors images already load fine without the attribute - but with it, any image whose server does not sendAccess-Control-Allow-Originwill fail to load entirely. That makes this a regression for Firefox users while only Safari (require-corp) benefits, since there a cross-origin image without CORP/CORS is blocked anyway and CORS mode is its only chance to load. The output buffer callback already knows$is_safari/ which COEP value it sent; only callcsme_add_crossorigin_to_images()in therequire-corpcase. (This is likely the same reason Gutenberg removed IMG handling in WordPress/gutenberg#76618 when moving to credentialless isolation.) -
Handle
srcset. An IMG whosesrcis same-origin but whosesrcsetincludes cross-origin candidates (or which has onlysrcset) is missed. Checking both attributes and adding the attribute when any candidate URL is cross-origin would match how core checks multiple attributes per tag (e.g. VIDEOsrc+poster). -
Rebase onto the current stack. #27 (7.1 function-name/init fixes) and #28 (HEIC module removal) touch
includes/cross-origin-isolation.phpand its neighbors; this branch is based on an older main. Please rebase once those merge - the@since 0.3.0tag can stay, matching the next release.
Happy to re-review after the update.
adamsilverstein
force-pushed
the
copilot/ensure-images-use-cors-attribute
branch
2 times, most recently
from
July 16, 2026 16:24
d33c65a to
1a53613
Compare
Owner
|
Applied the requested changes directly on this branch:
PHPCS clean; PHPUnit 66 tests passing locally (the 3 pre-existing header-assertion failures are the no-xdebug CLI environment issue, they pass in CI). Since I authored these changes, this PR needs a fresh pair of eyes rather than my re-review. |
adamsilverstein
approved these changes
Jul 16, 2026
This was referenced Jul 16, 2026
Core's wp_add_crossorigin_attributes() excludes IMG (removed in Gutenberg#76618), but under require-corp isolation on Safari, cross-origin images without CORP headers are blocked and a CORS request is their only chance to load. Delegate all other elements to core/Gutenberg, then add the attribute to IMG tags whose src or srcset references another origin. Only applies under require-corp: under credentialless (Firefox) images load fine without it, and forcing CORS mode would break images from servers that do not send Access-Control-Allow-Origin. Rebuilt on main past the 7.1 integration and v1.0.0 changes, which conflicted with every commit of the original branch history.
adamsilverstein
force-pushed
the
copilot/ensure-images-use-cors-attribute
branch
from
July 16, 2026 17:24
1a53613 to
d15471a
Compare
Owner
|
Rebuilt the branch on current main. Every commit in the old history conflicted with the WordPress 7.1 integration (#27) and v1.0.0 (#32) changes to
Verified locally: PHPCS clean; PHPUnit 37/40 with only the 3 known local-only The earlier CodeRabbit findings are resolved: the The JS-side counterpart (MutationObserver adds |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Gutenberg is removing
imgfrom the elements mutated bywp_add_crossorigin_attributes()/gutenberg_add_crossorigin_attributes()(gutenberg#76618). This plugin's COEP/COOP mode still requirescrossorigin="anonymous"on images for cross-origin isolation to work on Firefox/Safari.Changes
includes/cross-origin-isolation.php: Addcsme_add_crossorigin_to_images()— an IMG-only implementation usingWP_HTML_Tag_Processorthat addscrossorigin="anonymous"to cross-origin<img>elements. Other elements (AUDIO,LINK,SCRIPT,VIDEO,SOURCE) are still handled by core/Gutenberg.csme_start_coep_coop_output_buffer(): Now first delegates towp_add_crossorigin_attributes()/gutenberg_add_crossorigin_attributes()for non-IMG elements, then callscsme_add_crossorigin_to_images()to add back IMG support.tests/Test_Add_Crossorigin_Attributes.php: 9 test cases covering cross-origin/same-origin IMG URLs, relative paths, existing attributes, confirmation that non-IMG tags are not modified, and edge cases.Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.
Summary by CodeRabbit
Release Notes
Improvements
Tests