WAIT: Multiple css definitions with %s on one control: second definition receives the already substituted value (frontend only)

[UserfreundSuat]

This is a bug that has been present for some time, but I finially got myself to reporting it:

Bricks 2.4.2, includes/assets.php, generate_css_rules_from_setting().

When a control declares more than one css definition and more than one of them uses the %s placeholder, the frontend CSS of the second (and every following) definition is broken. The builder canvas renders it correctly, because the JS implementation substitutes per definition.

Control:

'css' => [
	[ 'property' => '--section-pt', 'value' => 'var(--section-pt-%s)' ],
	[ 'property' => '--section-pb', 'value' => 'var(--section-pb-%s)' ],
],

Setting value none. Frontend output:

#brxe-vwmavn {
	--section-pt: var(--section-pt-none);
	--section-pb: var(--section-pb-var(--section-pt-none));
}

Cause

$setting_value is set once before the foreach ( $css_definitions as $css_definition ) loop (line 2975). Inside the loop the substitution writes its result back into the same variable:

// line 3137
$setting_value = str_replace( '%s', $setting_value, $css_definition['value'] );

The same happens at line 3135 for definitions without %s ($setting_value = $css_definition['value']). From the second iteration on, %s is therefore replaced with the previous definition’s output instead of the raw setting value.

Suggested fix

Keep the raw setting value per control and work on a per-definition copy, for example:

$raw_setting_value = $setting_value;

foreach ( $css_definitions as $css_definition ) {
	$setting_value = $raw_setting_value;
	// ... existing code unchanged
}

or write the substituted value into a separate variable ($css_value) and use that for the rest of the iteration. This would make the PHP generator behave like the builder’s JS.

Best Regards
Suat

[Matej]

Hi @UserfreundSuat,

So, I’m not sure how to replicate it. I’ve added this filter in the child theme:

add_filter( 'bricks/elements/section/controls', function( $controls ) {
	$controls['reproSpacingToken'] = [
		'tab'            => 'content',
		'label'          => 'Repro spacing token',
		'type'           => 'text',
		'hasDynamicData' => false,
		'css'            => [
			[
				'property' => '--section-pt',
				'value'    => 'var(--section-pt-%s)',
			],
			[
				'property' => '--section-pb',
				'value'    => 'var(--section-pb-%s)',
			],
		],
	];

	return $controls;
} );

I’ve entered “none” as a valid and this is a result:

Can you post your control so I try with yours? Or if you can, can you try with my snippet?

Thank you,
Matej

[UserfreundSuat]

Hi @Matej ,

my Control is part of a custom Element, which is part of an abstract Element Class with which I extend your abstract Element class, so copying and pasting it would be a bit of a chore, to get all the none native stuff out.

But your code produces the exact same bug with the native section Element (I disabled my child-theme and added your code with a Code-Plugin). I was curious if this difference in behaviour was from you useing inline-Styles + Chrome, and me useing External-Files + Firefox, so I tested these different setups:

This is the generated CSS when useing external-files (Firefox):

And this is the generated CSS when useing inline-styles (Firefox):

This is the generated CSS when useing external-files (Chrome):

But Chrome hides the invalid value so I opened the CSS-Source to see the whole CSS-Rule:

This is how inline-styles are shown in (Chrome):

Same as with the external-files:

I also always reset the setting and generated new CSS-Assets for each of the screenshots.

Inside the Builder the inline-CSS is generated correctly (Chrome):

I’ll do some further digging, but I’m curious if you could try it also with External-Files asset enqueing or if you are useing internally some other form of building the frontend assets (I highly suspect you don’t but might aswell ask).

As I stated earlier I have encountered this issue multiple times, and always worked around it
so I’m really confused why you can’t reproduce the issue.

Best Regards
Suat

[UserfreundSuat]

Hi @Matej,

Your screenshot shows bricks-frontend-inline-css, so we are both looking at the frontend output, yet we get different results from the same control. Let’s take the browser and the asset mode out of the equation entirely and call the PHP generator directly. This snippet prints the generated rules as an HTML <pre>-Tag at the start of every frontend page:

add_action( 'bricks_body', function() {
	$controls = [
		'reproSpacingToken' => [
			'tab'   => 'content',
			'label' => 'Repro spacing token',
			'type'  => 'text',
			'css'   => [
				[ 'property' => '--section-pt', 'value' => 'var(--section-pt-%s)' ],
				[ 'property' => '--section-pb', 'value' => 'var(--section-pb-%s)' ],
			],
		],
	];

	$rules = \Bricks\Assets::generate_css_rules_from_setting(
		[ 'reproSpacingToken' => 'none' ],
		'reproSpacingToken',
		'none',
		$controls,
		'#brxe-repro',
		'content'
	);

	echo "\n<pre>Bricks " . BRICKS_VERSION . "\n" . print_r( $rules, true ) . "</pre>\n</div>\n";
} );

On Bricks 2.4.2 (public release) I get:

Bricks 2.4.2
Array
(
    [#brxe-repro] => Array
        (
            [0] => --section-pt: var(--section-pt-none)
            [1] => --section-pb: var(--section-pb-var(--section-pt-none))
        )

)

Could you run the same snippet and post your output together with the Bricks version it prints? If your build differs from the public 2.4.2, this might already be fixed on your side.

Best regards
Suat

[Matej]

Hi @UserfreundSuat,

Aha, I see. Thank you so much for sooo detailed and useful write up and additional tests!

I ran your snippet on Bricks 2.4.2 and got the same output, including --section-pb: var(--section-pb-var(--section-pt-none)). Meaning, I’ve now replicated the issue and I’ve added it to our internal bug tracker. We’ll update this thread once it’s fixed.

Thank you again for detailed bug report

Best regards,
Matej