[wp-trac] [WordPress Trac] #35956: Script's dependencies are always moved to header

[WordPress Trac]

#35956: Script's dependencies are always moved to header
------------------------------+------------------------
 Reporter:  stephenharris     |       Owner:  ocean90
     Type:  defect (bug)      |      Status:  reviewing
 Priority:  normal            |   Milestone:  4.5
Component:  Script Loader     |     Version:  trunk
 Severity:  normal            |  Resolution:
 Keywords:  needs-unit-tests  |     Focuses:
------------------------------+------------------------
Changes (by ocean90):

 * keywords:   => needs-unit-tests
 * owner:   => ocean90
 * status:  new => reviewing
 * milestone:  Awaiting Review => 4.5


Old description:

> I found this after the patch ([36604]) for #35973 was applied. I was
> going to report it there, but digging a little deeper I think this is a
> pre-existing bug which has only been brought to light.
>
> I'm seeing a that all dependencies of registered script are being moved
> to the header, even when the script that depends on them is loaded in the
> footer. It's late here, so I'm going to post this example and follow with
> some unit tests tomorrow.
>
> Suppose we have:
>
> {{{
> wp_register_script('parent','parent.js', array('child'), '1', true);
> wp_register_script('child', 'child.js', array(), '1', true);
> }}}
>
> Then we have the trace:
>
> 1. `WP_Scripts->do_head_items()`
> 2. `WP_Dependencies->do_items(false,0)`
> 3. `WP_Scripts->all_deps( $handles )` (we loose all reference to the
> group here, defaults to false)
>
> At this point we loop through the handles and call
> `WP_Scripts->set_group( $handle, $recursion, $group )`.
>
> `$group` here is ''always'' false, at least when printing header scripts.
> Now lets assume we are at handle 'parent' in our loop. We then call
> `WP_Scripts->all_deps` (and so `WP_Dependencies->all_deps`), passing the
> dependencies as the first argument (e.g. `array( 'child' )`), the second
> argument (`$recursion`) as `true` (naturally), and the final argument
> `false` still.
>
> That loops over the dependencies and we call
>
> {{{
> WP_Scripts->set_group( 'child', true, $group )`
> }}}
>
> As noted `$group` is always false (and cast to 0), so with the following,
> the 'child' script's group is always set to 0
>
> {{{
> if ( $recursion ) {
>         $group = min( $this->group, $group );
> }
> }}}
>
> My first thoughts are that at some point `$group` should really be set to
> the parent's group (e.g. 1 in this case).

New description:

 I found this after the patch ([36604]) for #35873 was applied. I was going
 to report it there, but digging a little deeper I think this is a pre-
 existing bug which has only been brought to light.

 I'm seeing a that all dependencies of registered script are being moved to
 the header, even when the script that depends on them is loaded in the
 footer. It's late here, so I'm going to post this example and follow with
 some unit tests tomorrow.

 Suppose we have:

 {{{
 wp_register_script('parent','parent.js', array('child'), '1', true);
 wp_register_script('child', 'child.js', array(), '1', true);
 }}}

 Then we have the trace:

 1. `WP_Scripts->do_head_items()`
 2. `WP_Dependencies->do_items(false,0)`
 3. `WP_Scripts->all_deps( $handles )` (we loose all reference to the group
 here, defaults to false)

 At this point we loop through the handles and call `WP_Scripts->set_group(
 $handle, $recursion, $group )`.

 `$group` here is ''always'' false, at least when printing header scripts.
 Now lets assume we are at handle 'parent' in our loop. We then call
 `WP_Scripts->all_deps` (and so `WP_Dependencies->all_deps`), passing the
 dependencies as the first argument (e.g. `array( 'child' )`), the second
 argument (`$recursion`) as `true` (naturally), and the final argument
 `false` still.

 That loops over the dependencies and we call

 {{{
 WP_Scripts->set_group( 'child', true, $group )`
 }}}

 As noted `$group` is always false (and cast to 0), so with the following,
 the 'child' script's group is always set to 0

 {{{
 if ( $recursion ) {
         $group = min( $this->group, $group );
 }
 }}}

 My first thoughts are that at some point `$group` should really be set to
 the parent's group (e.g. 1 in this case).

--

--
Ticket URL: <https://core.trac.wordpress.org/ticket/35956#comment:2>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform