diff --git a/concat-css.php b/concat-css.php index 18024d8..d3ca77b 100644 --- a/concat-css.php +++ b/concat-css.php @@ -35,6 +35,15 @@ function __construct( $styles ) { ); } + protected function has_inline_style( $handle ) { + $after_output = $this->get_data( $handle, 'after' ); + if ( ! empty( $after_output ) ) { + return true; + } + + return false; + } + function do_items( $handles = false, $group = false ) { $handles = false === $handles ? $this->queue : (array) $handles; $stylesheets = array(); @@ -42,32 +51,55 @@ function do_items( $handles = false, $group = false ) { $this->all_deps( $handles ); - $stylesheet_group_index = 0; - // Merge CSS into a single file - $concat_group = 'concat'; - // Concat group on top (first array element gets processed earlier) - $stylesheets[ $concat_group ] = array(); + $concat_group = null; foreach ( $this->to_do as $key => $handle ) { + if ( ! isset( $this->registered[ $handle ] ) ) { + unset( $this->to_do[ $key ] ); + continue; + } + $obj = $this->registered[ $handle ]; - $obj->src = apply_filters( 'style_loader_src', $obj->src, $obj->handle ); + + if ( empty( $obj->src ) ) { + if ( null !== $concat_group ) { + $stylesheets[] = $concat_group; + $concat_group = null; + } + + $stylesheets[] = array( + 'type' => 'do_item', + 'handle' => $handle, + ); + unset( $this->to_do[ $key ] ); + continue; + } + + $css_url = apply_filters( 'style_loader_src', $obj->src, $obj->handle ); // Core is kind of broken and returns "true" for src of "colors" handle // http://core.trac.wordpress.org/attachment/ticket/16827/colors-hacked-fixed.diff // http://core.trac.wordpress.org/ticket/20729 - $css_url = $obj->src; - if ( 'colors' == $obj->handle && true === $css_url ) { + if ( 'colors' === $obj->handle && true === $css_url ) { $css_url = wp_style_loader_src( $css_url, $obj->handle ); } - $css_url_parsed = parse_url( $obj->src ); + // If a filter returns something unexpected, let's not concat it. + if ( ! is_string( $css_url ) || '' === $css_url ) { + $css_url_parsed = false; + $css_path = ''; + } else { + $css_url_parsed = parse_url( $css_url ); + $css_path = ( is_array( $css_url_parsed ) && isset( $css_url_parsed['path'] ) ) ? $css_url_parsed['path'] : ''; + } + $extra = $obj->extra; // Don't concat by default $do_concat = false; // Only try to concat static css files - if ( false !== strpos( $css_url_parsed['path'], '.css' ) ) { + if ( $css_path && false !== strpos( $css_path, '.css' ) ) { $do_concat = true; } else { if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { @@ -123,12 +155,13 @@ function do_items( $handles = false, $group = false ) { } // Allow plugins to disable concatenation of certain stylesheets. - if ( $do_concat && ! apply_filters( 'css_do_concat', $do_concat, $handle ) ) { + $filtered_concat = apply_filters( 'css_do_concat', $do_concat, $handle ); + if ( $do_concat && ! $filtered_concat ) { if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { echo sprintf( "\n\n", esc_html( $handle ) ); } } - $do_concat = apply_filters( 'css_do_concat', $do_concat, $handle ); + $do_concat = $filtered_concat; if ( true === $do_concat ) { $media = $obj->args; @@ -136,26 +169,56 @@ function do_items( $handles = false, $group = false ) { $media = 'all'; } - $stylesheets[ $concat_group ][ $media ][ $handle ] = $css_url_parsed['path']; + if ( null !== $concat_group && $concat_group['media'] !== $media ) { + $stylesheets[] = $concat_group; + $concat_group = null; + } + + if ( null === $concat_group ) { + $concat_group = array( + 'type' => 'concat', + 'media' => $media, + 'paths' => array(), + 'handles' => array(), + ); + } + + $concat_group['paths'][] = $css_path; + $concat_group['handles'][] = $handle; $this->done[] = $handle; + + if ( $this->has_inline_style( $handle ) ) { + $stylesheets[] = $concat_group; + $concat_group = null; + } } else { - $stylesheet_group_index ++; - $stylesheets[ $stylesheet_group_index ]['noconcat'][] = $handle; - $stylesheet_group_index ++; + if ( null !== $concat_group ) { + $stylesheets[] = $concat_group; + $concat_group = null; + } + $stylesheets[] = array( + 'type' => 'do_item', + 'handle' => $handle, + ); } unset( $this->to_do[ $key ] ); } - foreach ( $stylesheets as $idx => $stylesheets_group ) { - foreach ( $stylesheets_group as $media => $css ) { - if ( 'noconcat' == $media ) { - foreach ( $css as $handle ) { - if ( $this->do_item( $handle, $group ) ) { - $this->done[] = $handle; - } - } - continue; - } elseif ( count( $css ) > 1 ) { + if ( null !== $concat_group ) { + $stylesheets[] = $concat_group; + } + + foreach ( $stylesheets as $css_array ) { + if ( 'do_item' === $css_array['type'] ) { + if ( $this->do_item( $css_array['handle'], $group ) ) { + $this->done[] = $css_array['handle']; + } + } elseif ( 'concat' === $css_array['type'] && isset( $css_array['paths'] ) ) { + $media = $css_array['media']; + $css = $css_array['paths']; + $handles = $css_array['handles']; + + if ( count( $css ) > 1 ) { $fs_paths = array(); foreach ( $css as $css_uri_path ) { $fs_paths[] = $this->dependency_path_mapping->uri_path_to_fs_path( $css_uri_path ); @@ -178,17 +241,24 @@ function do_items( $handles = false, $group = false ) { $href = $siteurl . "/_static/??" . $path_str; } else { - $href = Page_Optimize_Utils::cache_bust_mtime( current( $css ), $siteurl ); + $href = Page_Optimize_Utils::cache_bust_mtime( $css[0], $siteurl ); } - $handles = array_keys( $css ); $css_id = "$media-css-" . md5( $href ); if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) { - echo apply_filters( 'page_optimize_style_loader_tag', "\n", $handles, $href, $media ); + $tag = "\n"; } else { - echo apply_filters( 'page_optimize_style_loader_tag', "\n", $handles, $href, $media ); + $tag = "\n"; } - array_map( array( $this, 'print_inline_style' ), array_keys( $css ) ); + + $tag = apply_filters( 'page_optimize_style_loader_tag', $tag, $handles, $href, $media ); + + if ( is_array( $handles ) && count( $handles ) === 1 ) { + $tag = apply_filters( 'style_loader_tag', $tag, $handles[0], $href, $media ); + } + + echo $tag; + array_map( array( $this, 'print_inline_style' ), $handles ); } } diff --git a/tests/test_css_concat_eligibility.php b/tests/test_css_concat_eligibility.php index ef6d8a8..f67c2bb 100644 --- a/tests/test_css_concat_eligibility.php +++ b/tests/test_css_concat_eligibility.php @@ -186,4 +186,84 @@ public function test_css_do_concat_filter_can_disable_concatenation(): void { remove_filter( 'css_do_concat', $filter_callback, 10 ); } } + + /** + * Verifies that style_loader_src mutations do not accumulate when a handle + * falls back to core's do_item(). + * + * We allow style_loader_src to run multiple times, but Page Optimize must NOT + * overwrite $registered[handle]->src with the filtered URL. Otherwise, core will + * re-filter an already-filtered URL and non-idempotent filters will stack. + */ + public function test_style_loader_src_does_not_accumulate_for_non_concatenated_handle(): void { + $application_count = 0; + + // Deliberately non-idempotent: appends a NEW po_filter param every time it runs. + // If the input URL was already mutated (contains po_filter=1), a second run will + // produce po_filter=1&po_filter=2, which we can detect. + $filter_callback = function( $src, $handle ) use ( &$application_count ) { + if ( 'a' !== $handle ) { + return $src; + } + + $application_count++; + + $sep = ( false === strpos( $src, '?' ) ) ? '?' : '&'; + return $src . $sep . 'po_filter=' . $application_count; + }; + + add_filter( 'style_loader_src', $filter_callback, 10, 2 ); + + // Force 'a' to fall through to core do_item(). + $exclude_filter = function( $do_concat, $handle ) { + return ( 'a' === $handle ) ? false : $do_concat; + }; + add_filter( 'css_do_concat', $exclude_filter, 10, 2 ); + + try { + $styles = $this->new_concat_styles(); + + $a = $this->make_content_css( 'po-double-filter-a.css' ); + $styles->add( 'a', $a, [], null, 'all' ); + $styles->enqueue( 'a' ); + + // Capture the original stored src. If Page Optimize mutates $obj->src, this will change. + $original_src = $styles->registered['a']->src; + + $html = $this->render( $styles ); + + // Precondition: confirm it rendered via core do_item (not a Page Optimize-generated ID). + $this->assertMatchesRegularExpression( '/id=[\'"]a-css[\'"]/', $html, 'Expected core do_item output for excluded handle.' ); + + // Primary assertion: the registered src must remain unmodified. + $this->assertSame( + $original_src, + $styles->registered['a']->src, + 'Page Optimize must not overwrite $registered[handle]->src with the filtered URL (causes accumulated mutations).' + ); + + // Extract href for handle 'a' from the rendered link tag. + $this->assertMatchesRegularExpression( '/data-handles=[\'"]a[\'"]/', $html, 'Expected data-handles="a" in output.' ); + + preg_match( '/data-handles=[\'"]a[\'"][^>]*href=[\'"]([^\'"]+)[\'"]/', $html, $m ); + $this->assertNotEmpty( $m[1], 'Could not extract href for handle a.' ); + + // Decode & / & etc. for reliable query parsing. + $href = html_entity_decode( $m[1], ENT_QUOTES ); + + // If mutations accumulated, we'd see po_filter twice (po_filter=1&po_filter=2). + preg_match_all( '/(?:\?|&)po_filter=/', $href, $mm ); + $this->assertSame( + 1, + count( $mm[0] ), + 'Expected exactly one po_filter param in final href. Multiple occurrences indicate accumulated mutations across filter applications.' + ); + + // Supplementary: confirm the filter ran; the test is valid even if it ran more than once. + $this->assertGreaterThanOrEqual( 1, $application_count, 'Expected style_loader_src filter to run at least once.' ); + } finally { + remove_filter( 'style_loader_src', $filter_callback, 10 ); + remove_filter( 'css_do_concat', $exclude_filter, 10 ); + } + } } diff --git a/tests/test_css_concat_order.php b/tests/test_css_concat_order.php index 30126e6..6f66d62 100644 --- a/tests/test_css_concat_order.php +++ b/tests/test_css_concat_order.php @@ -240,8 +240,8 @@ public function test_same_media_stylesheets_concatenate_within_run(): void { * need special handling and cannot be concatenated. * * Enqueue order: a (local) -> b (rtl-marked) -> c (local) - * Expected output: [a], [b], [c] (three separate tags, in order) - * Bug: [a], [c], [b], [b] ( RTL stylesheet pushed to end, also not sure why there are 2 - test harness issue? ) + * Expected output: [a], [b], [b], [c] (two tags for b: base + RTL, in order) + * Bug: [a], [c], [b], [b] (RTL stylesheet pushed to end) * * @group css-order-bug */ @@ -268,10 +268,10 @@ public function test_rtl_stylesheet_breaks_concat_run_and_preserves_order(): voi $groups = $this->extract_handle_groups( $html ); $handles = $this->flatten_groups( $groups ); - $this->assertSame( [ 'a', 'b', 'c' ], $handles, 'RTL stylesheet must not cause reordering.' ); + $this->assertSame( [ 'a', 'b', 'b', 'c' ], $handles, 'RTL stylesheet must not cause reordering.' ); // Each should be in its own group (no concatenation across RTL boundary). - $this->assertSame( [ [ 'a' ], [ 'b' ], [ 'c' ] ], $groups, 'RTL stylesheet should break concat run.' ); + $this->assertSame( [ [ 'a' ], [ 'b' ], [ 'b' ], [ 'c' ] ], $groups, 'RTL stylesheet should break concat run.' ); } /**