Make WordPress Core

Opened 6 years ago

Closed 4 years ago

#44484 closed defect (bug) (fixed)

Mediaelement scripts forced loaded in header

Reported by: themezly's profile Themezly Owned by: joemcgill's profile joemcgill
Milestone: 5.3 Priority: normal
Severity: major Version: 4.9.6
Component: Media Keywords: has-patch
Focuses: performance Cc:

Description

Before this https://make.wordpress.org/core/2017/10/30/mediaelement-upgrades-in-wordpress-4-9/

media element scripts were loading where called

example:

wp_enqueue_script( 'wp-mediaelement' ); // loads above site-scripts in footer with dependencies 

/*
mediaelement-and-player.min.js
wp-mediaelement.min.js
*/

wp_enqueue_script( 'site-scripts'); // loads in footer as needed

now

mediaelement.min.js
mediaelement-core.min.js

load in head and wp-mediaelement.min.js loads in footer

If I call them one by one without calling wp-mediaelement the order is kept

wp_enqueue_script( 'mediaelement-migrate' );// loads in footer
wp_enqueue_script( 'mediaelement-core' );// loads in footer
wp_enqueue_script( 'site-scripts'); // loads in footer

if you look here it is intended for all of them to be in footer

https://github.com/WordPress/WordPress/blob/6a9a5e123c49862babf664367c26c89499fed4e0/wp-includes/script-loader.php#L377-L378

https://github.com/WordPress/WordPress/blob/6a9a5e123c49862babf664367c26c89499fed4e0/wp-includes/script-loader.php#L474

but wp_enqueue_script( 'wp-mediaelement' );

depends on wp_enqueue_script( 'mediaelement' ); http://prntscr.com/k15288

https://github.com/WordPress/WordPress/blob/6a9a5e123c49862babf664367c26c89499fed4e0/wp-includes/script-loader.php#L376

which loads in header only

so we can never move the scripts to footer where they should be

If I set the mediaelement to load in footer everything works as intended.

Attachments (1)

44484.diff (900 bytes) - added by joemcgill 4 years ago.

Download all attachments as: .zip

Change History (10)

#1 @SergeyBiryukov
6 years ago

  • Component changed from General to Media

#2 @joemcgill
5 years ago

  • Focuses performance added
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.3

Hi @Themezly,

Thanks for the report. This does seem to be an unintentional change from [41877]. I'm going to set this to the next major release, but we might be able to put it out in a 5.2.x release, if it's ready.

This ticket was mentioned in Slack in #core-media by mike. View the logs.


5 years ago

#4 @kirasong
5 years ago

  • Owner set to joemcgill
  • Status changed from new to assigned

#5 @kirasong
4 years ago

@joemcgill I'm noticing this one doesn't have a patch yet, and it's almost beta 3.

Would you like to do this for 5.3 still, or should we move it to 5.4 or Future Release?

This ticket was mentioned in Slack in #core-media by mike. View the logs.


4 years ago

@joemcgill
4 years ago

#7 @joemcgill
4 years ago

  • Keywords needs-testing has-patch added; needs-patch removed

44484.diff ensures mediaelement is loaded in the footer. Before this change, all mediaelement related scripts are loading in the header on the post edit screen. After this change, they are loaded in the footer.

#8 @desrosj
4 years ago

  • Keywords needs-testing removed

@joemcgill 44484.diff looks good. I tested in Classic and block editors and didn't see any issues.

#9 @joemcgill
4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 46379:

Media: Ensure medialement scripts are loaded in the footer.

This fixes a regression in [41877] which caused mediaelement scripts to load in the header.

Props Themezly.
Fixes #44484.

Note: See TracTickets for help on using tickets.