Make WordPress Core

Changeset 36871


Ignore:
Timestamp:
03/06/2016 07:49:54 PM (9 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.