WordPress.org

Make WordPress Core

Opened 11 months ago

Last modified 5 months ago

#51837 reviewing defect (bug)

Adding `add_theme_support` for html5 for scripts, does not remove the type attribute from `wp-emoji-release.min.js`

Reported by: kanlukasz Owned by: SergeyBiryukov
Milestone: 5.9 Priority: normal
Severity: normal Version: 5.3
Component: Script Loader Keywords: has-patch
Focuses: Cc:

Description

This is a follow-up to #42804.

Using: add_theme_support('html5', ['script', 'style']); removes the type attribute from my custom scripts but does not remove it from the default wp-emoji-release.min.js

Eg.:
<script src="https://example.com/wp-includes/js/wp-emoji-release.min.js?ver=5.5.3" type="text/javascript" defer=""></script>

Attachments (3)

51837.diff (5.0 KB) - added by tferry 8 months ago.
Removes the type attribute from emoji scripts and zxcvbn-async.js when theme supports html5 script tags
51837.2.diff (3.8 KB) - added by tferry 8 months ago.
Updated patch without changes to the package-lock.json file
51837.3.diff (3.6 KB) - added by tferry 8 months ago.
Removes emoji_type_attr filter

Download all attachments as: .zip

Change History (9)

#1 @peterwilsoncc
11 months ago

  • Version changed from 5.5.3 to 5.3

Hello @kanlukasz and welcome to trac!

I've been able to confirm the report, the script is loaded using JavaScript in emoji-loader.js and the theme support setting is not passed to the JavaScript to determine if the type attribute should be included.

From /src/js/_enqueues/lib/emoji-loader.js:

function addScript( src ) {
        var script = document.createElement( 'script' );

        script.src = src;
        script.defer = script.type = 'text/javascript';
        document.getElementsByTagName( 'head' )[0].appendChild( script );
}

At a quick glance the same issue applies to src/js/_enqueues/lib/zxcvbn-async.js if it's loaded on the front-end of a site. Some external libraries may include the type too but unfortunately they are out of WordPress's control.

@tferry
8 months ago

Removes the type attribute from emoji scripts and zxcvbn-async.js when theme supports html5 script tags

#2 @SergeyBiryukov
8 months ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 5.8
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

Thanks for the patch! I don't think the emoji_type_attr filter is needed here, just checking current_theme_supports( 'html5', 'script' ) should be enough. Looks good to me otherwise.

@tferry
8 months ago

Updated patch without changes to the package-lock.json file

#3 @tferry
8 months ago

Thanks @SergeyBiryukov! I've also refreshed the patch to remove the changes to the package-lock.json file - not sure how that snuck in...

@tferry
8 months ago

Removes emoji_type_attr filter

This ticket was mentioned in Slack in #core by chaion07. View the logs.


5 months ago

#5 @chaion07
5 months ago

Thanks to @kanlukasz for reporting this issue. We reviewed this ticket during a recent [bug-scrub session]https://wordpress.slack.com/archives/C02RQBWTW/p1623096832360300. With Beta 1 coming in a day, checking with @SergeyBiryukov to kindly draw some attention to this. Thank you!

#6 @hellofromTonya
5 months ago

  • Milestone changed from 5.8 to 5.9

Today is 5.8 Beta 1. Ran out of time to land this one. Looks really close to ready pending code review. Punting to 5.9.

Note: See TracTickets for help on using tickets.