WordPress.org
Get WordPress

Make WordPress Core

Changeset 36871


Ignore:
Timestamp:
03/06/2016 07:49:54 PM (10 years ago)
Author:
ocean90
Message:

Dependencies: Improve group processing of script dependencies.

This is a follow-up to [36604].

When processing dependencies $this->group will be the minimum of the script's registered group and all preceding siblings. This is wrong because only a scripts ancestors in the dependency chain should affect where it is loaded. Effectively $this->group introduced a form of global state which potentially corrupted the group of dependencies. Sorting covers up this problem.
The issue in #35873 was that script were not moving their dependencies to a lower group when necessary.

The fix:

  • In WP_Dependencies::all_deps() pass the new $group value to WP_Dependencies::all_deps(). Previously the wrong value was passed because the parent script could have moved with WP_Scripts::set_group().
  • In WP_Scripts::all_deps() pass the $group parameter to WP_Dependencies::all_deps() so it doesn't always use false for $group. Same for WP_Styles::all_deps().

Props stephenharris, gitlost.
Fixes #35956.

Location:
trunk
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/wp-includes/class.wp-dependencies.php

    r36744 r36871  
    7878         * @access public
    7979         * @since 2.8.0
     80         * @deprecated 4.5.0
    8081         * @var int
    8182         */
    … …  
    162163                                continue;
    163164
    164                         $moved = $this->set_group( $handle, $recursion, $group );
     165                        $moved     = $this->set_group( $handle, $recursion, $group );
     166                        $new_group = $this->groups[ $handle ];
    165167
    166168                        if ( $queued && !$moved ) // already queued and in the right group
    … …  
    172174                        elseif ( $this->registered[$handle]->deps && array_diff($this->registered[$handle]->deps, array_keys($this->registered)) )
    173175                                $keep_going = false; // Item requires dependencies that don't exist.
    174                         elseif ( $this->registered[$handle]->deps && !$this->all_deps( $this->registered[$handle]->deps, true, $group ) )
     176                        elseif ( $this->registered[$handle]->deps && !$this->all_deps( $this->registered[$handle]->deps, true, $new_group ) )
    175177                                $keep_going = false; // Item requires dependencies that don't exist.
    176178
    … …  
    398400                $group = (int) $group;
    399401
    400                 if ( $recursion ) {
    401                         $group = min( $this->group, $group );
    402                 }
    403 
    404                 $this->group = $group;
    405 
    406                 if ( isset($this->groups[$handle]) && $this->groups[$handle] <= $group )
    407                         return false;
    408 
    409                 $this->groups[$handle] = $group;
     402                if ( isset( $this->groups[ $handle ] ) && $this->groups[ $handle ] <= $group ) {
     403                        return false;
     404                }
     405
     406                $this->groups[ $handle ] = $group;
     407
    410408                return true;
    411409        }

  • trunk/src/wp-includes/class.wp-scripts.php

    r36744 r36871  
    510510         */
    511511        public function all_deps( $handles, $recursion = false, $group = false ) {
    512                 $r = parent::all_deps( $handles, $recursion );
     512                $r = parent::all_deps( $handles, $recursion, $group );
    513513                if ( ! $recursion ) {
    514514                        /**

  • trunk/src/wp-includes/class.wp-styles.php

    r36744 r36871  
    311311         */
    312312        public function all_deps( $handles, $recursion = false, $group = false ) {
    313                 $r = parent::all_deps( $handles, $recursion );
     313                $r = parent::all_deps( $handles, $recursion, $group );
    314314                if ( ! $recursion ) {
    315315                        /**

  • trunk/tests/phpunit/tests/dependencies/scripts.php

    r36633 r36871  
    273273         */
    274274        function test_wp_register_script_with_dependencies_in_head_and_footer() {
    275                 wp_register_script( 'parent', '/parent.js', array( 'child' ), '1', true ); // in footer
    276                 wp_register_script( 'child', '/child.js', array( 'grandchild' ), '1', false ); // in head
    277                 wp_register_script( 'grandchild', '/grandchild.js', array(), '1', true ); // in footer
     275                wp_register_script( 'parent', '/parent.js', array( 'child-head' ), null, true ); // in footer
     276                wp_register_script( 'child-head', '/child-head.js', array( 'child-footer' ), null, false ); // in head
     277                wp_register_script( 'child-footer', '/child-footer.js', array(), null, true ); // in footer
    278278
    279279                wp_enqueue_script( 'parent' );
    … …  
    282282                $footer = get_echo( 'wp_print_footer_scripts' );
    283283
    284                 $expected_header  = "<script type='text/javascript' src='/grandchild.js?ver=1'></script>\n";
    285                 $expected_header .= "<script type='text/javascript' src='/child.js?ver=1'></script>\n";
    286                 $expected_footer  = "<script type='text/javascript' src='/parent.js?ver=1'></script>\n";
     284                $expected_header  = "<script type='text/javascript' src='/child-footer.js'></script>\n";
     285                $expected_header .= "<script type='text/javascript' src='/child-head.js'></script>\n";
     286                $expected_footer  = "<script type='text/javascript' src='/parent.js'></script>\n";
     287
     288                $this->assertEquals( $expected_header, $header );
     289                $this->assertEquals( $expected_footer, $footer );
     290        }
     291
     292        /**
     293         * @ticket 35956
     294         */
     295        function test_wp_register_script_with_dependencies_in_head_and_footer_in_reversed_order() {
     296                wp_register_script( 'child-head', '/child-head.js', array(), null, false ); // in head
     297                wp_register_script( 'child-footer', '/child-footer.js', array(), null, true ); // in footer
     298                wp_register_script( 'parent', '/parent.js', array( 'child-head', 'child-footer' ), null, true ); // in footer
     299
     300                wp_enqueue_script( 'parent' );
     301
     302                $header = get_echo( 'wp_print_head_scripts' );
     303                $footer = get_echo( 'wp_print_footer_scripts' );
     304
     305                $expected_header  = "<script type='text/javascript' src='/child-head.js'></script>\n";
     306                $expected_footer  = "<script type='text/javascript' src='/child-footer.js'></script>\n";
     307                $expected_footer .= "<script type='text/javascript' src='/parent.js'></script>\n";
     308
     309                $this->assertEquals( $expected_header, $header );
     310                $this->assertEquals( $expected_footer, $footer );
     311        }
     312
     313        /**
     314         * @ticket 35956
     315         */
     316        function test_wp_register_script_with_dependencies_in_head_and_footer_in_reversed_order_and_two_parent_scripts() {
     317                wp_register_script( 'grandchild-head', '/grandchild-head.js', array(), null, false ); // in head
     318                wp_register_script( 'child-head', '/child-head.js', array(), null, false ); // in head
     319                wp_register_script( 'child-footer', '/child-footer.js', array( 'grandchild-head' ), null, true ); // in footer
     320                wp_register_script( 'child2-head', '/child2-head.js', array(), null, false ); // in head
     321                wp_register_script( 'child2-footer', '/child2-footer.js', array(), null, true ); // in footer
     322                wp_register_script( 'parent-footer', '/parent-footer.js', array( 'child-head', 'child-footer', 'child2-head', 'child2-footer' ), null, true ); // in footer
     323                wp_register_script( 'parent-header', '/parent-header.js', array( 'child-head' ), null, false ); // in head
     324
     325                wp_enqueue_script( 'parent-footer' );
     326                wp_enqueue_script( 'parent-header' );
     327
     328                $header = get_echo( 'wp_print_head_scripts' );
     329                $footer = get_echo( 'wp_print_footer_scripts' );
     330
     331                $expected_header  = "<script type='text/javascript' src='/child-head.js'></script>\n";
     332                $expected_header .= "<script type='text/javascript' src='/grandchild-head.js'></script>\n";
     333                $expected_header .= "<script type='text/javascript' src='/child2-head.js'></script>\n";
     334                $expected_header .= "<script type='text/javascript' src='/parent-header.js'></script>\n";
     335
     336                $expected_footer  = "<script type='text/javascript' src='/child-footer.js'></script>\n";
     337                $expected_footer .= "<script type='text/javascript' src='/child2-footer.js'></script>\n";
     338                $expected_footer .= "<script type='text/javascript' src='/parent-footer.js'></script>\n";
    287339
    288340                $this->assertEquals( $expected_header, $header );
    … …  
    383435
    384436        /**
    385          * @ticket 14853-2
     437         * @ticket 14853
    386438         */
    387439        public function test_wp_add_inline_script_before_with_concat() {
Note: See TracChangeset for help on using the changeset viewer.