Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 43 additions & 15 deletions src/wp-includes/media.php
Original file line number Diff line number Diff line change
Expand Up @@ -6736,24 +6736,38 @@ function wp_add_crossorigin_attributes( string $html ): string {
'SOURCE' => array( 'src' ),
);

while ( $processor->next_tag() ) {
/*
* Name of the bookmark on the AUDIO or VIDEO element the cursor is inside of,
* for as long as that element still needs the attribute. Closing tags are
* visited so the bookmark cannot outlive its element and be mistaken for the
* parent of a later SOURCE.
*/
$media_bookmark = null;

while ( $processor->next_tag( array( 'tag_closers' => 'visit' ) ) ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m a bit confused on the purpose of the change to visit closing tags, and the use of $media_bookmark and set_bookmark() — it’s worth an explanatory comment.

note that the HTML API provides WP_HTML_Tag_Processor->has_bookmark() which indicates if a bookmark has been set, so it appears like creating $media_bookmark is unnecessary.

is the goal to start investigating upon entering an AUDIO or VIDEO element, and then kinda of resetting once exiting one?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing @dmsnell!

Good tip on has_bookmark, I'll keep that in mind for the future.

For now, I'm closing this PR in favor of WordPress/gutenberg#82614 which removes the HTML processing entirely (ref).

@adamsilverstein adamsilverstein Sep 10, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on your questions @dmsnell - the answer to the last one is yes, and it turns out that was the only part worth keeping.

Claude went back over this one, here is where it landed:

Enter on an AUDIO or VIDEO opener, reset on its closer. That was the whole
intent, and nothing at the loop said so, which is a fair thing to have been
confused by.

The has_bookmark() note is right too. $media_bookmark only ever held the
literal 'audio-video-parent', so it was a string carrying a boolean's worth
of information - has_bookmark() and release_bookmark() would have said the
same thing without the variable.

$tag = $processor->get_tag();

if ( ! isset( $cross_origin_tag_attributes[ $tag ] ) ) {
continue;
}
$crossorigin = $processor->get_attribute( 'crossorigin' );
if ( null !== $crossorigin ) {
continue;
}

if ( 'AUDIO' === $tag || 'VIDEO' === $tag ) {
$processor->set_bookmark( 'audio-video-parent' );
$media_bookmark = null;

if ( ! $processor->is_tag_closer() && null === $processor->get_attribute( 'crossorigin' ) ) {
$media_bookmark = 'audio-video-parent';
$processor->set_bookmark( $media_bookmark );
}
}

$processor->set_bookmark( 'resume' );
if ( $processor->is_tag_closer() ) {
continue;
}

$sought = false;
$crossorigin = $processor->get_attribute( 'crossorigin' );
if ( null !== $crossorigin ) {
continue;
}

$is_cross_origin = false;

Expand All @@ -6770,18 +6784,32 @@ function wp_add_crossorigin_attributes( string $html ): string {

if ( $is_cross_origin ) {
if ( 'SOURCE' === $tag ) {
$sought = $processor->seek( 'audio-video-parent' );
if ( null !== $media_bookmark ) {
$processor->set_bookmark( 'resume' );

/*
* A seek past MAX_SEEK_OPS leaves the cursor where it is, so mark the
* parent only once the cursor has reached it. Marking a SOURCE does
* nothing for CORS and would hide the media element that was missed.
*/
if ( $processor->seek( $media_bookmark ) ) {
$processor->set_attribute( 'crossorigin', 'anonymous' );
$processor->seek( 'resume' );
}

if ( $sought ) {
$processor->set_attribute( 'crossorigin', 'anonymous' );
/*
* The element is either marked or past the seek budget, which does not
* come back, so its remaining SOURCE children need nothing.
*/
$media_bookmark = null;
Comment thread
adamsilverstein marked this conversation as resolved.
}
} else {
$processor->set_attribute( 'crossorigin', 'anonymous' );
}

if ( $sought ) {
$processor->seek( 'resume' );
$processor->release_bookmark( 'audio-video-parent' );
// A marked AUDIO or VIDEO needs nothing from its SOURCE children.
if ( 'AUDIO' === $tag || 'VIDEO' === $tag ) {
$media_bookmark = null;
}
}
}
}
Expand Down
52 changes: 52 additions & 0 deletions tests/phpunit/tests/media/wpCrossOriginIsolation.php
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ public function test_set_flag_preserves_upload_media_module_dependencies() {
* Verifies that cross-origin elements get crossorigin="anonymous" added.
*
* @ticket 64766
* @ticket 65930
*
* @runInSeparateProcess
* @preserveGlobalState disabled
Expand Down Expand Up @@ -409,6 +410,9 @@ public function data_elements_that_should_get_crossorigin() {
'cross-origin source inside video' => array(
'<video><source src="https://external.example.com/video.mp4" type="video/mp4" /></video>',
),
'multiple cross-origin sources' => array(
'<video><source src="https://external.example.com/video.mp4" type="video/mp4" /><source src="https://external.example.com/video.webm" type="video/webm" /></video>',
),
);
}

Expand All @@ -420,6 +424,7 @@ public function data_elements_that_should_get_crossorigin() {
* in credentialless mode without needing explicit CORS headers.
*
* @ticket 64766
* @ticket 65930
*
* @runInSeparateProcess
* @preserveGlobalState disabled
Expand All @@ -442,6 +447,47 @@ public function test_output_buffer_does_not_add_crossorigin( $html ) {
$this->assertStringNotContainsString( 'crossorigin="anonymous"', $output );
}

/**
* A failed seek() must not leave the crossorigin attribute on the SOURCE element.
*
* WP_HTML_Tag_Processor::seek() returns false without moving the cursor once
* MAX_SEEK_OPS is exceeded. The cursor is then still on the SOURCE, so marking
* the element without checking the return value adds the attribute where it does
* nothing for CORS and leaves the parent media element unmarked.
*
* @ticket 65930
*
* @covers ::wp_add_crossorigin_attributes
*/
public function test_source_is_not_marked_when_seeking_the_parent_fails() {
$this->setExpectedIncorrectUsage( 'WP_HTML_Tag_Processor::seek' );

/*
* Marking a media element costs two seeks, one back to the parent and one
* forward to resume, so one more than half the budget outlasts it.
*/
$media_elements = intdiv( WP_HTML_Tag_Processor::MAX_SEEK_OPS, 2 ) + 1;

$output = wp_add_crossorigin_attributes(
str_repeat(
'<video><source src="https://external.example.com/video.mp4" /></video>',
$media_elements
)
);

$this->assertSame(
0,
preg_match_all( '/<source\b[^>]*\bcrossorigin\b/i', $output ),
'A SOURCE element must never receive the crossorigin attribute.'
);

$this->assertSame(
$media_elements - 1,
substr_count( $output, 'crossorigin="anonymous"' ),
'Every media element reachable within the seek budget should have been marked.'
);
}

/**
* Data provider for elements that should not receive crossorigin="anonymous".
*
Expand All @@ -461,6 +507,12 @@ public function data_elements_that_should_not_get_crossorigin() {
'relative URL script' => array(
'<script src="/wp-includes/js/wp-embed.min.js"></script>',
),
'source outside a media element' => array(
'<picture><source src="https://external.example.com/image.avif" /><img src="/local-image.jpg" /></picture>',
),
'source in a video with crossorigin' => array(
'<audio src="/local-audio.mp3"></audio><video crossorigin="use-credentials"><source src="https://external.example.com/video.mp4" /></video>',
),
);
}

Expand Down
Loading