Media: Scope the crossorigin bookmark to its media element

[itzmekhokan]

A <source> element could hand crossorigin="anonymous" to the wrong media element, or raise _doing_it_wrong() from inside the cross-origin isolation output buffer.

wp_add_crossorigin_attributes() tracks the AUDIO or VIDEO parent of a SOURCE with an audio-video-parent bookmark that is never scoped to the element it was set on, and releases it after the first cross-origin SOURCE. A second SOURCE then seeks a bookmark that no longer exists, and a SOURCE outside any media element walks back to an unrelated one. This visits closing tags so the bookmark cannot outlive its element, keeps it across sibling sources, and only seeks a bookmark the loop set.

The accepted set only narrows, and the loop now performs fewer seek() calls than before.

Trac ticket: https://core.trac.wordpress.org/ticket/65930

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: diagnosing the bookmark scoping fault, implementing the fix, and extending the existing data providers. All changes were reviewed and validated by me.

[github-actions]

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props khokansardar, adamsilverstein.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The 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

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[adamsilverstein]

@itzmekhokan - were you able to reproduce the bug? Are the two examples in your test enough to cause the whitescreen if present in content?

[adamsilverstein]

Claude went through the seek path here, findings below:

WP_HTML_Tag_Processor::seek() returns false without moving the cursor once MAX_SEEK_OPS (1000) is passed. With the return value unchecked, the cursor is still on the SOURCE when set_attribute() runs, so crossorigin lands on the SOURCE - where it does nothing for CORS - and the parent media element is left unmarked. Trunk guarded this with $sought.

Reproduced locally with intdiv( WP_HTML_Tag_Processor::MAX_SEEK_OPS, 2 ) + 1 copies of <video><source src="https://external.example.com/video.mp4" /></video>. Marking a parent costs two seeks, so the last pair goes over budget: the attribute winds up on the <source>, and seek() raises _doing_it_wrong() from inside the output buffer - the same failure this PR is fixing.

Suggested change

	$processor->set_bookmark( 'resume' );
	$processor->seek( $media_bookmark );
	$processor->set_attribute( 'crossorigin', 'anonymous' );
	$processor->seek( 'resume' );
	// The element is marked, so further SOURCE children need nothing.
	$media_bookmark = null;
	$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 actually reached it. Marking
	* a SOURCE does nothing for CORS and would hide the missed parent.
	*/
	if ( $processor->seek( $media_bookmark ) ) {
	$processor->set_attribute( 'crossorigin', 'anonymous' );
	$processor->seek( 'resume' );
	// The element is marked, so further SOURCE children need nothing.
	$media_bookmark = null;
	}

A test that fails without the guard and passes with it:

	/**
	 * 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.'
		);
	}

With both applied the class is green here, 30 tests and 38 assertions.

One thing the guard does not cover: a seek over budget still raises _doing_it_wrong() inside the display handler, so a page with 500+ marked media elements can still take the white screen. That is trunk's behavior too, so probably its own follow up rather than something for this PR.

Does that hold up on your end?

[itzmekhokan]

Yes, reproduced — guard pushed as 7456c6a.

Are the two examples enough? No. The _doing_it_wrong() only becomes a white screen if an error handler is installed that itself buffers or throws. Same markup, trunk, WP_DEBUG on:

error handler	result
none	notice discarded, page renders fine
calls ob_start()	Fatal error: ob_start(): Cannot use output buffering in output buffering display handlers
throws ErrorException	Fatal error: Uncaught ErrorException: …seek was called incorrectly. Unknown bookmark name.

Which is probably why @b0b3k couldn't bisect it — the plugin installing the handler isn't the one printing the <source>.

Your seek finding: holds. 501 videos, trunk marks 0 sources, my PR marked 1. Fixed, with one tweak — $media_bookmark = null needs to stay outside the if. seek_count never resets, so once the budget is gone every remaining <source> just retries: 16 notices instead of 6. Also dropped the bookmark when a media element is marked from its own src, where it was seeking back to re-set an attribute already there.

Your test is in as written, green single + multisite. Diffed trunk against the patch over 110 markup shapes: 99 identical, 11 differ and all 11 are this fix. One behavior note — <video><audio src="/local.mp3"></audio><source src="cdn"></video> marks the audio on trunk and marks nothing now; a trailing <source> is ignored by the browser anyway, so unmarked beats wrong element.

Agree the >500 case is a separate follow-up. It's removable if it's worth it — a lookahead pass instead of seeking back, no bookmarks or seek() in the buffer at all.