From 00e87b40c87945da83d1a8c403702777b66316fc Mon Sep 17 00:00:00 2001 From: "Dominik Schilling (ocean90)" Date: Sun, 6 Mar 2016 19:49:54 +0000 Subject: [PATCH] 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. git-svn-id: https://develop.svn.wordpress.org/trunk@36871 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/class.wp-dependencies.php | 16 +++-- src/wp-includes/class.wp-scripts.php | 2 +- src/wp-includes/class.wp-styles.php | 2 +- tests/phpunit/tests/dependencies/scripts.php | 66 +++++++++++++++++--- 4 files changed, 68 insertions(+), 18 deletions(-) diff --git a/src/wp-includes/class.wp-dependencies.php b/src/wp-includes/class.wp-dependencies.php index 11068f970a..5a1a9286e7 100644 --- a/src/wp-includes/class.wp-dependencies.php +++ b/src/wp-includes/class.wp-dependencies.php @@ -77,6 +77,7 @@ class WP_Dependencies { * * @access public * @since 2.8.0 + * @deprecated 4.5.0 * @var int */ public $group = 0; @@ -161,7 +162,8 @@ class WP_Dependencies { if ( in_array($handle, $this->done, true) ) // Already done continue; - $moved = $this->set_group( $handle, $recursion, $group ); + $moved = $this->set_group( $handle, $recursion, $group ); + $new_group = $this->groups[ $handle ]; if ( $queued && !$moved ) // already queued and in the right group continue; @@ -171,7 +173,7 @@ class WP_Dependencies { $keep_going = false; // Item doesn't exist. elseif ( $this->registered[$handle]->deps && array_diff($this->registered[$handle]->deps, array_keys($this->registered)) ) $keep_going = false; // Item requires dependencies that don't exist. - elseif ( $this->registered[$handle]->deps && !$this->all_deps( $this->registered[$handle]->deps, true, $group ) ) + elseif ( $this->registered[$handle]->deps && !$this->all_deps( $this->registered[$handle]->deps, true, $new_group ) ) $keep_going = false; // Item requires dependencies that don't exist. if ( ! $keep_going ) { // Either item or its dependencies don't exist. @@ -397,16 +399,12 @@ class WP_Dependencies { public function set_group( $handle, $recursion, $group ) { $group = (int) $group; - if ( $recursion ) { - $group = min( $this->group, $group ); + if ( isset( $this->groups[ $handle ] ) && $this->groups[ $handle ] <= $group ) { + return false; } - $this->group = $group; + $this->groups[ $handle ] = $group; - if ( isset($this->groups[$handle]) && $this->groups[$handle] <= $group ) - return false; - - $this->groups[$handle] = $group; return true; } diff --git a/src/wp-includes/class.wp-scripts.php b/src/wp-includes/class.wp-scripts.php index 0b3f922ed0..655c07f23d 100644 --- a/src/wp-includes/class.wp-scripts.php +++ b/src/wp-includes/class.wp-scripts.php @@ -509,7 +509,7 @@ class WP_Scripts extends WP_Dependencies { * @return bool True on success, false on failure. */ public function all_deps( $handles, $recursion = false, $group = false ) { - $r = parent::all_deps( $handles, $recursion ); + $r = parent::all_deps( $handles, $recursion, $group ); if ( ! $recursion ) { /** * Filter the list of script dependencies left to print. diff --git a/src/wp-includes/class.wp-styles.php b/src/wp-includes/class.wp-styles.php index 788e8a3fa9..a175058baa 100644 --- a/src/wp-includes/class.wp-styles.php +++ b/src/wp-includes/class.wp-styles.php @@ -310,7 +310,7 @@ class WP_Styles extends WP_Dependencies { * @return bool True on success, false on failure. */ public function all_deps( $handles, $recursion = false, $group = false ) { - $r = parent::all_deps( $handles, $recursion ); + $r = parent::all_deps( $handles, $recursion, $group ); if ( ! $recursion ) { /** * Filter the array of enqueued styles before processing for output. diff --git a/tests/phpunit/tests/dependencies/scripts.php b/tests/phpunit/tests/dependencies/scripts.php index 02045cf448..a8525032cc 100644 --- a/tests/phpunit/tests/dependencies/scripts.php +++ b/tests/phpunit/tests/dependencies/scripts.php @@ -272,18 +272,70 @@ class Tests_Dependencies_Scripts extends WP_UnitTestCase { * @ticket 35873 */ function test_wp_register_script_with_dependencies_in_head_and_footer() { - wp_register_script( 'parent', '/parent.js', array( 'child' ), '1', true ); // in footer - wp_register_script( 'child', '/child.js', array( 'grandchild' ), '1', false ); // in head - wp_register_script( 'grandchild', '/grandchild.js', array(), '1', true ); // in footer + wp_register_script( 'parent', '/parent.js', array( 'child-head' ), null, true ); // in footer + wp_register_script( 'child-head', '/child-head.js', array( 'child-footer' ), null, false ); // in head + wp_register_script( 'child-footer', '/child-footer.js', array(), null, true ); // in footer wp_enqueue_script( 'parent' ); $header = get_echo( 'wp_print_head_scripts' ); $footer = get_echo( 'wp_print_footer_scripts' ); - $expected_header = "\n"; - $expected_header .= "\n"; - $expected_footer = "\n"; + $expected_header = "\n"; + $expected_header .= "\n"; + $expected_footer = "\n"; + + $this->assertEquals( $expected_header, $header ); + $this->assertEquals( $expected_footer, $footer ); + } + + /** + * @ticket 35956 + */ + function test_wp_register_script_with_dependencies_in_head_and_footer_in_reversed_order() { + wp_register_script( 'child-head', '/child-head.js', array(), null, false ); // in head + wp_register_script( 'child-footer', '/child-footer.js', array(), null, true ); // in footer + wp_register_script( 'parent', '/parent.js', array( 'child-head', 'child-footer' ), null, true ); // in footer + + wp_enqueue_script( 'parent' ); + + $header = get_echo( 'wp_print_head_scripts' ); + $footer = get_echo( 'wp_print_footer_scripts' ); + + $expected_header = "\n"; + $expected_footer = "\n"; + $expected_footer .= "\n"; + + $this->assertEquals( $expected_header, $header ); + $this->assertEquals( $expected_footer, $footer ); + } + + /** + * @ticket 35956 + */ + function test_wp_register_script_with_dependencies_in_head_and_footer_in_reversed_order_and_two_parent_scripts() { + wp_register_script( 'grandchild-head', '/grandchild-head.js', array(), null, false ); // in head + wp_register_script( 'child-head', '/child-head.js', array(), null, false ); // in head + wp_register_script( 'child-footer', '/child-footer.js', array( 'grandchild-head' ), null, true ); // in footer + wp_register_script( 'child2-head', '/child2-head.js', array(), null, false ); // in head + wp_register_script( 'child2-footer', '/child2-footer.js', array(), null, true ); // in footer + wp_register_script( 'parent-footer', '/parent-footer.js', array( 'child-head', 'child-footer', 'child2-head', 'child2-footer' ), null, true ); // in footer + wp_register_script( 'parent-header', '/parent-header.js', array( 'child-head' ), null, false ); // in head + + wp_enqueue_script( 'parent-footer' ); + wp_enqueue_script( 'parent-header' ); + + $header = get_echo( 'wp_print_head_scripts' ); + $footer = get_echo( 'wp_print_footer_scripts' ); + + $expected_header = "\n"; + $expected_header .= "\n"; + $expected_header .= "\n"; + $expected_header .= "\n"; + + $expected_footer = "\n"; + $expected_footer .= "\n"; + $expected_footer .= "\n"; $this->assertEquals( $expected_header, $header ); $this->assertEquals( $expected_footer, $footer ); @@ -382,7 +434,7 @@ class Tests_Dependencies_Scripts extends WP_UnitTestCase { } /** - * @ticket 14853-2 + * @ticket 14853 */ public function test_wp_add_inline_script_before_with_concat() { global $wp_scripts;