Make WordPress Core

Opened 8 years ago

Last modified 5 years ago

#36448 new defect (bug)

When concatenating scripts in script-loader dependencies may not be honoured.

Reported by: gitlost's profile gitlost Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.4.2
Component: Script Loader Keywords:
Focuses: Cc:

Description

This is a follow on from 36392#comment:13. Ticket #15833 looks to be related.

When concatenating scripts, dependencies may not be honoured for scripts that trigger the use of $print_html (ie those outside the default directories or having conditionals or (if available!) inline scripts).

For instance this test where three depends on one and one has a conditional:

<?php
public function test_wp_script_conditional_concat_dependency() {
        global $wp_scripts;

        $wp_scripts->do_concat = true;
        $wp_scripts->default_dirs = array( '/directory/' );
        $wp_scripts->default_version = 1;

        wp_enqueue_script( 'one', '/directory/one.js', array(), 1 );
        wp_enqueue_script( 'two', '/directory/two.js', array(), 1 );
        wp_enqueue_script( 'three', '/directory/three.js', array( 'one' ), 1 );

        wp_script_add_data( 'one', 'conditional', 'blah' );

        $wp_print_scripts = get_echo( 'wp_print_scripts' );
        $print_scripts = get_echo( '_print_scripts' );

        $expected  = "<!--[if blah]>\n";
        $expected .= "<script type='text/javascript' src='/directory/one.js?ver=1'></script>\n";
        $expected .= "<![endif]-->\n";
        $expected .= "<script type='text/javascript' src='/wp-admin/load-scripts.php?c=0&amp;load%5B%5D=two,three&amp;ver=1'></script>\n";

        $this->assertEquals( $expected, $print_scripts );
        $this->assertEquals( '', $wp_print_scripts );
}

will fail, as the conditional stuff will always be outputted after the concatenated scripts, resulting in one appearing after three:

<script type='text/javascript' src='/wp-admin/load-scripts.php?c=0&amp;load%5B%5D=two,three&amp;ver=1'></script>
<!--[if blah]>
<script type='text/javascript' src='/directory/one.js?ver=1'></script>
<![endif]-->

The same is also true, mutatis mutandis (ahem), for styles, but I'll open a separate ticket for that.

Attachments (1)

36448.batch.3.patch (28.0 KB) - added by gitlost 8 years ago.
Version of "batch" demo with better treatment of before/after inline code.

Download all attachments as: .zip

Change History (2)

@gitlost
8 years ago

Version of "batch" demo with better treatment of before/after inline code.

#1 @gitlost
8 years ago

The above patch is version 3 (see #36392 "batch" patches) of a way around this, that puts the concatenate data into batches ordered by dependency, and then outputs each batch in order.

It includes all the previous tests plus some extra.

It also includes the jquery-core tests by @jeremyfelt (36392#comment:19), for which this version of the batching has been changed to pass, and shows that my 36392#comment:14 re being unwanted behaviour was false.

Note: See TracTickets for help on using tickets.