Make WordPress Core

Opened 6 years ago

Closed 7 months ago

Last modified 7 months ago

#43248 closed enhancement (duplicate)

wp_localize_script() and ultimately print_extra_script() not using script_loader_tag filter

Reported by: csmillie's profile csmillie Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.9.4
Component: Script Loader Keywords: good-first-bug has-patch
Focuses: javascript Cc:

Description

When you add a localized value like:

<?php
    wp_localize_script( "scripts", "localized", 'value' );

Generates

This value is added to wp_script->localized and then outputted using print_extra_script() which directly echo's the code without using the script_loader_tag filter.

I'd suggest adjusting print_extra_script() to use the script_loader_tag filter. I'm unclear if a new filter should be created but I think the existing was added to filter code like the localize additions.

I created a patch and I'm open to contributing if it's acceptable:

<?php
diff --git a/wp-includes/class.wp-scripts.php b/wp-includes/class.wp-scripts.php
index a9299f0..b770734 100644
--- a/wp-includes/class.wp-scripts.php
+++ b/wp-includes/class.wp-scripts.php
@@ -202,13 +202,25 @@ class WP_Scripts extends WP_Dependencies {
                if ( !$echo )
                        return $output;

-               echo "<script type='text/javascript'>\n"; // CDATA and type='text/javascript' is not needed for HTML 5
-               echo "/* <![CDATA[ */\n";
-               echo "$output\n";
-               echo "/* ]]> */\n";
-               echo "</script>\n";
+               $tag = "<script type='text/javascript'>\n"; // CDATA and type='text/javascript' is not needed for HTML 5
+               $tag .= "/* <![CDATA[ */\n";
+               $tag .= "$output\n";
+               $tag .= "/* ]]> */\n";
+               $tag .= "</script>\n";
+
+               /**
+                 * Filters the HTML script tag of an enqueued script.
+                 *
+                 * @since 4.1.0
+                 *
+                 * @param string $tag    The `<script>` tag for the enqueued script.
+                 * @param string $handle The script's registered handle.
+                 * @param string $src    The script's source URL.
+                 */
+                $tag = apply_filters( 'script_loader_tag', $tag, $handle, $output );
+
+               echo $tag;

-               return true;
        }

        /**

Attachments (2)

new-filter-script-tag.PNG (49.7 KB) - added by muhammadfaizanhaidar 3 years ago.
Added filter in print_extra_script function
resolved-phpcss-unit-tests-issues.png (173.2 KB) - added by muhammadfaizanhaidar 3 years ago.
resolved-phpcss-unit-tests-issues

Download all attachments as: .zip

Change History (12)

#1 follow-up: @ocean90
6 years ago

  • Component changed from I18N to Script Loader
  • Focuses accessibility removed

Hello @csmillie, welcome to WordPress Trac!

Thanks for your request. It looks like your proposed change wouldn't be backward compatible because current plugins expect a URL for the $src argument and not raw JavaScript. Maybe it should be left empty instead. What do you think? And what's your use case for the filter at this place?

#2 in reply to: ↑ 1 @csmillie
6 years ago

Thanks for the prompt feedback.

You are correct the original filter defines the last parameter as a URL but I think using the JS output is consistent with the context and use case for the tag.

The filter allows adjustments to the tag surrounding the URL ( in the original definition ) or JS output ( as I've used ). This seems to be a natural extension to me but could be breaking as you point out.

I'm unclear if a new filter should be defined or the existing filter ( script_loader_tag ) should be re-defined/updated to note this change?

Thanks again.

Replying to ocean90:

Hello @csmillie, welcome to WordPress Trac!

Thanks for your request. It looks like your proposed change wouldn't be backward compatible because current plugins expect a URL for the $src argument and not raw JavaScript. Maybe it should be left empty instead. What do you think? And what's your use case for the filter at this place?

#3 @ShortPixel
5 years ago

I have a use case for this. We're using the filter to add this attribute:

data-cfasync="false"

to our JS so that it's not asynced out by Cloudflare. The problem is that the script is localized and it depends on the localized variables but the localized part is not caught by the filter so the main script needs to wait anyway for the async part... The solution for us would be any method to add the data-cfasync attribute also to the localized script section.

#4 @swissspidy
4 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to Future Release

I think a new filter, e.g. inline_script_tag, would be best to avoid any BC issues. Passing the tag and the handle, and maybe the content (raw JS) too.

This ticket was mentioned in PR #1759 on WordPress/wordpress-develop by MuhammadFaizanHaidar.


3 years ago
#5

  • Keywords has-patch added; needs-patch removed

Added new filter in "print_extra_script" function.

Description:
Added new filter in "print_extra_script" function. Passed tag, handle and raw js as parameters.
/wp-includes/class.wp-scripts.php b/wp-includes/class.wp-scripts.php ( LN 244 - LN 266 )

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

@muhammadfaizanhaidar
3 years ago

Added filter in print_extra_script function

@muhammadfaizanhaidar
3 years ago

resolved-phpcss-unit-tests-issues

davidwebca commented on PR #1759:


21 months ago
#6

I've needed this a few times in the past and thought it was coming. Is it just about passing tests and finishing merge? 👀

MuhammadFaizanHaidar commented on PR #1759:


21 months ago
#7

I've needed this a few times in the past and thought it was coming. Is it just about passing tests and finishing merge? 👀

Hi thanks for making me recall this I totally forgot when I made this merge request and what were issues and you are welcome to help thanks!

#8 @xiven
7 months ago

I found myself writing a near-identical patch to this one just a few days ago (though I had named the filter "extra_script_loader_tag"), and then when considering submitting it, I discovered your merge request.

I would very much like to see this get merged - it would be extremely useful to me, and no doubt others. I'm not sure what (if anything) is currently blocking it though. Is there anything we can do to help?

My use case for it relates to implementing a proper Content Security Policy on a WordPress site, since this patch makes it possible for a theme or plugin to add CSP nonces to all enqueued scripts programatically, including those generated by wp_localize_script().

#9 @swissspidy
7 months ago

  • Milestone Future Release deleted
  • Resolution set to duplicate
  • Status changed from new to closed

This is solved in WordPress 6.4 by [56687] / #58664 as print_extra_script() now uses wp_print_inline_script_tag() which has a wp_inline_script_attributes filter. CSP is one use case.

See https://make.wordpress.org/core/2023/10/17/script-loading-changes-in-wordpress-6-4/ for more information.

#10 @xiven
7 months ago

That is excellent news. Thank you for the informative reply.

Note: See TracTickets for help on using tickets.