WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 months ago

#34290 reopened defect (bug)

Not possible to have a percent symbol contained in the default value of a theme_mod

Reported by: kuus Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.4
Component: Customize Keywords: needs-patch
Focuses: javascript, administration Cc:

Description

For instance after having added a setting like this

$wp_customize->add_setting( 'my-id', array(
  'default' => '10%',
  'transport' => 'postMessage'
));

in the customize screen source code (in the settings JSON) is printed:

s["my-id"] = {"value":"10","transport":"postMessage","dirty":false};

the % is stripped out. The line responsible for this is: http://git.io/vCz2R with the sprintf function. I don't know what could be a good fix to this. Many thanks

Change History (15)

#1 in reply to: ↑ description @SergeyBiryukov
3 years ago

Hi @kuus, welcome to Trac!

Thanks for the report. Have you tried escaping the value with another %, e.g. 'default' => '10%%'?

#2 @kuus
3 years ago

Hello @SergeyBiryukov, yes I thought about that but I hoped there could have been a better solution. Mainly because I am using, inside a custom control class, the $setting->default which differs from the $setting->value() even though no valued have been stored to the database. Tell me if you need more clarification. I would also like to know what @westonruter thinks about this.

#3 @moonomo
3 years ago

Just a note, I had to use option instead of theme_mod due to this for a particular setting. My use case was to introduce some predefined macro for user to input for Footer String i.e 'Copyright © %CUR_YEAR%.'. Although, it could have #CUR_YEAR# or something else, theme_mod went crazy for the % without saying anything about it.

#4 follow-up: @westonruter
3 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Interesting issue. I'm not sure if this is something that should be accounted for in the Customizer or if it is just an idiosyncrasy for how theme mods work. Since it is get_theme_mod() that is doing the sprintf(), then I'm inclined to the former latter. The only alternative I can think of is for the Customizer to explicitly str_replace( '%', '%%', $setting->default) when it is accessing a theme mod's default value, but that could have other unintended consequences.

Would be worth a patch to explore what can be done.

#5 @kuus
3 years ago

To me the best would be to fix this issue in theme_mods API directly since that is where the problem lies. So, first of all, it might be a stupid question, but I don't understand why that sprintf is there in the get_theme_mod function. Is it really needed?

#6 @davetgreen
2 years ago

@westonruter @kuus It might be worth perform a regex on the string and only carry out the string replacement if the % character comes immediately after a number?

#7 @kuus
2 years ago

@davetgreen I don't think the regex is the way to go, because the percent symbol could be anywhere in a string, it does not necessarily comes after a number.

I did a bit of research in the codebase to see where this sprintf was introduced and it seems since the beginning (2007)...here the commit that introduced the get_theme_mod function and the theme.php at that time. @matt it seems that you wrote that function (not sure if you are the same Matt though) I am still wondering why this sprintf, it seems to me that we could just get rid of it and solve the bug this way..what do you think?

Last edited 2 years ago by kuus (previous) (diff)

#8 @ocean90
2 years ago

#36952 was marked as a duplicate.

#9 @kuus
4 months ago

I've re-stumbled upon this issue but now the problem is almost inverse than the ticket description. Instead of having the % symbol stripped out now it is doubled..

so for instance after having added a setting like this

<?php
$wp_customize->add_setting( 'my-id', array(
  'default' => '10%',
  'transport' => 'postMessage'
));

in the customize screen source code (in the settings JSON) is printed:

s["my-id"] = {"value":"10%%","transport":"postMessage","dirty":false,"type":"theme_mod"};

..any idea? perhaps @westonruter ?

#10 @kuus
4 months ago

  • Resolution set to invalid
  • Status changed from new to closed

I think I was doing something wrong..closing as not an issue anymore apparently.

#11 @ocean90
3 months ago

  • Milestone Future Release deleted

#12 @SergeyBiryukov
2 months ago

#43824 was marked as a duplicate.

#13 in reply to: ↑ 4 @SergeyBiryukov
2 months ago

  • Milestone set to Awaiting Review
  • Resolution invalid deleted
  • Status changed from closed to reopened

Apparently still an issue, per #43824.

Replying to westonruter:

The only alternative I can think of is for the Customizer to explicitly str_replace( '%', '%%', $setting->default) when it is accessing a theme mod's default value, but that could have other unintended consequences.

Related: [24630], #21241.

#14 @daviedR
2 months ago

Facing the same issue here, my case is:

get_theme_mod( 'mykey', '100%' );

when mykey has no value, the function would return '100' instead of '100%'. The '%' character is stripped out because of the sprintf function.

In the function description:

If the modification name does not exist, then the $default will be passed through sprintf() PHP function with the first string the template directory URI and the second string the stylesheet directory URI.

Is there any reason why WordPress use sprintf to convert first and second %s into template and stylesheet directory URI?

#15 @aristath
2 months ago

Most people that have issues with this is because we're working with CSS values and need to be able to use %. Right now this is what the get_theme_mod function uses:

	if ( is_string( $default ) ) {
		$default = sprintf( $default, get_template_directory_uri(), get_stylesheet_directory_uri() );
	}

The sprintf there is useful... I've used it to set default values for image theme-mods where I need the default value to be an image contained in the theme and I'm too bored to type get_template_directory_url. I'm not saying it's right... It's just fast and it works.

Why don't we simply change it to this:

	if ( is_string( $default ) && ( false !== strpos( $default, '%s' ) || false !== strpos( $default, '%1&s' ) || false !== strpos( $default, '%2$s' ) ) ) {
		$default = sprintf( $default, get_template_directory_uri(), get_stylesheet_directory_uri() );
	}

f we don't want to strip % symbols then checking for %s, %1$s and %2$s before sprintf looks like a reasonable and safe solution. This way sprintf will only run if we've got a syntax where it actually makes sense to use sprintf. Otherwise this is a really annoying bug...

Last edited 2 months ago by aristath (previous) (diff)
Note: See TracTickets for help on using tickets.