Media: Scope the crossorigin bookmark to its media element - #13221
Media: Scope the crossorigin bookmark to its media element#13221itzmekhokan wants to merge 2 commits into
Conversation
`wp_add_crossorigin_attributes()` tracked the AUDIO or VIDEO element a SOURCE belongs to with a bookmark that was never scoped to the element it was set on, and released it after the first cross-origin SOURCE. A second SOURCE then sought a bookmark that no longer existed, raising `_doing_it_wrong()` from inside the output buffer's display handler, while a SOURCE outside any media element walked back to an unrelated one and marked that instead. Visit closing tags so the bookmark cannot outlive its element, keep it across sibling SOURCE elements, and only seek a bookmark this loop set. Fixes #65930.
|
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. |
|
@itzmekhokan - were you able to reproduce the bug? Are the two examples in your test enough to cause the whitescreen if present in content? |
`WP_HTML_Tag_Processor::seek()` returns false without moving the cursor once `MAX_SEEK_OPS` is exceeded, so marking the parent without checking the return value puts `crossorigin` on the SOURCE, where it does nothing for CORS, and leaves the media element unmarked. The bookmark is dropped whether or not the seek lands, since the budget never comes back and every remaining SOURCE of that element would only raise the same notice again. A media element marked from its own attribute drops it as well, rather than having its SOURCE children seek back to re-set an attribute that is already there. See #65930.
|
Yes, reproduced — guard pushed as 7456c6a. Are the two examples enough? No. The
Which is probably why @b0b3k couldn't bisect it — the plugin installing the handler isn't the one printing the Your seek finding: holds. 501 videos, trunk marks 0 sources, my PR marked 1. Fixed, with one tweak — 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 — 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 |
That lowers the severity of the issue significantly although it still seems worth fixing. |
|
cc: @dmsnell for feedback on the HTML Processor code, double checking this isn't an API issue. |
adamsilverstein
left a comment
adamsilverstein
left a comment
There was a problem hiding this comment.
The code looks good to me and the tests are helpful. I'm researching whether we still need this code and if there is a better solution here.
|
Reviewing this again, I hjave a suspicion we don't need this I'm going to open some separate PRs proposing that and if they test well we can eliminate this entire class of errors. cc: @swissspidy |
|
@itzmekhokan I think we can remove this processing entirely, can you check if this PR resolves the issue? If so, please close out this PR in favor of that one: |
2 participants
A
<source>element could handcrossorigin="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 anaudio-video-parentbookmark 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.