-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Scripts: Use HTML API to build SCRIPT tags #10639
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scripts: Use HTML API to build SCRIPT tags #10639
Conversation
…html-api-for-script-tags
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. |
Co-authored-by: Weston Ruter <westonruter@gmail.com>
…html-api-for-script-tags
|
I'd like to land #10728 first so it's clear that the attribute handling is unchanged from trunk in this change. |
dmsnell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great end to a long journey. I’m glad to see it come in and I think it will have a major uplift in the reliability of scripts, especially when viewed in the progression of module support, SCRIPT escaping, attribute recognition, etc…
Thanks for another intentional step forward with WordPress’s SCRIPT story.
westonruter
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments.
…pi-for-script-tags
In some conditions the script tag contents are unsafe and cannot be set. Add a `_doing_it_wrong()` message. Add tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors wp_get_script_tag() and wp_get_inline_script_tag() to use the HTML API (WP_HTML_Tag_Processor) instead of string concatenation, providing automatic escaping of dangerous JavaScript/JSON content and safer SCRIPT tag generation.
Changes:
- Replaced string-based tag generation with
WP_HTML_Tag_Processorfor safer HTML construction - Added error handling in
wp_get_inline_script_tag()to return empty string when content cannot be safely embedded - Added test case for dangerous unescapeable script content
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/wp-includes/script-loader.php | Refactored wp_get_script_tag() and wp_get_inline_script_tag() to use HTML API with duplicate attribute handling and safe content embedding |
| tests/phpunit/tests/dependencies/wpInlineScriptTag.php | Added test for failure case when script content cannot be safely escaped |
Comments suppressed due to low confidence (1)
src/wp-includes/script-loader.php:1
- The documentation states 'an empty script tag with the provided attributes will be returned' but the implementation actually returns an empty string
''(line 2996). The documentation should be corrected to say 'an empty string will be returned' to match the actual behavior.
<?php
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple nits, but re-approving
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Script tags have complicated and unintuitive parsing rules that make them difficult to author correctly. The HTML API automatically escapes script tag contents as necessary and will set attributes correctly. Using the HTML API to generate SCRIPT tags improves safety when working with SCRIPT tags, resolving a class of issues that have manifested repeatedly. Changeset [61418] applied the HTML API to generate style tags in a similar way. Developed in #10639. Props jonsurrell, dmsnell, westonruter. Fixes #64500. See #64419, #40737, #62797, #63851, #51159. git-svn-id: https://develop.svn.wordpress.org/trunk@61485 602fd350-edb4-49c9-b593-d223f7449a82
Script tags have complicated and unintuitive parsing rules that make them difficult to author correctly. The HTML API automatically escapes script tag contents as necessary and will set attributes correctly. Using the HTML API to generate SCRIPT tags improves safety when working with SCRIPT tags, resolving a class of issues that have manifested repeatedly. Changeset [61418] applied the HTML API to generate style tags in a similar way. Developed in WordPress/wordpress-develop#10639. Props jonsurrell, dmsnell, westonruter. Fixes #64500. See #64419, #40737, #62797, #63851, #51159. Built from https://develop.svn.wordpress.org/trunk@61485 git-svn-id: http://core.svn.wordpress.org/trunk@60797 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Script tags have complicated and unintuitive parsing rules that make them difficult to author correctly. The HTML API automatically escapes script tag contents as necessary and will set attributes correctly. Using the HTML API to generate SCRIPT tags improves safety when working with SCRIPT tags, resolving a class of issues that have manifested repeatedly. Changeset [61418] applied the HTML API to generate style tags in a similar way. Developed in WordPress/wordpress-develop#10639. Props jonsurrell, dmsnell, westonruter. Fixes #64500. See #64419, #40737, #62797, #63851, #51159. Built from https://develop.svn.wordpress.org/trunk@61485 git-svn-id: https://core.svn.wordpress.org/trunk@60797 1a063a9b-81f0-0310-95a4-ce76da25c4cd
✅
Includes #10635(merged in r61477).Trac ticket: https://core.trac.wordpress.org/ticket/64500
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.