Skip to content

Fix mutation of $obj->src with style_loader_src filter #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mreishus wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Fix mutation of $obj->src with style_loader_src filter #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b63b4b4
Fix CSS concatenation to preserve original stylesheet order
dsas Jan 28, 2026
779df6d
Add defensive isset() check for paths array
dsas Jan 28, 2026
4ed57f6
Fix CSS concat ordering across media and inline boundaries
mreishus Jan 30, 2026
1515fc8
Fix running css_do_concat twice
mreishus Jan 30, 2026
1315ff6
Fix mutation of $obj->src with style_loader_src filter
mreishus Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 101 additions & 31 deletions concat-css.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,39 +35,71 @@ 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();
$siteurl = apply_filters( 'page_optimize_site_url', $this->base_url );

$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 ) {
Expand Down Expand Up @@ -123,39 +155,70 @@ 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<!-- No Concat CSS %s => Filtered `false` -->\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;
if ( empty( $media ) ) {
$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 );
Expand All @@ -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', "<link data-handles='" . esc_attr( implode( ',', $handles ) ) . "' rel='stylesheet' id='$css_id' href='$href' type='text/css' media='$media' />\n", $handles, $href, $media );
$tag = "<link data-handles='" . esc_attr( implode( ',', $handles ) ) . "' rel='stylesheet' id='$css_id' href='$href' type='text/css' media='$media' />\n";
} else {
echo apply_filters( 'page_optimize_style_loader_tag', "<link rel='stylesheet' id='$css_id' href='$href' type='text/css' media='$media' />\n", $handles, $href, $media );
$tag = "<link rel='stylesheet' id='$css_id' href='$href' type='text/css' media='$media' />\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 );
}
}

Expand Down
80 changes: 80 additions & 0 deletions tests/test_css_concat_eligibility.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 &amp; / &#038; 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 );
}
}
}
8 changes: 4 additions & 4 deletions tests/test_css_concat_order.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <link> 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 <link> tags for b: base + RTL, in order)
* Bug: [a], [c], [b], [b] (RTL stylesheet pushed to end)
*
* @group css-order-bug
*/
Expand All @@ -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.' );
}

/**
Expand Down
Loading