WordPress.org

Make WordPress Core

Opened 17 months ago

Last modified 3 months ago

#22781 new defect (bug)

setUserSetting cannot handle hyphens

Reported by: nacin Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.7
Component: Options, Meta APIs Keywords: 3.6-early
Focuses: Cc:

Description

sanitize_key() can, setUserSetting should be able to as well. It's not that sanitize_key() plays a role, but it sets expectations for what is allowed in keys across core. Both keys and values should be able to accept a hyphen.

The post-thumbnail image size cannot be remembered, for example, if it is added to image_size_names_choose.

Change History (5)

comment:1 nacin17 months ago

  • Keywords 3.6-early added

comment:2 jond3r17 months ago

  • Cc jond3r@… added

Note that setUserSetting() is a JavaScript function (in utils.js).

I also also found a similar PHP function: set_user_setting() (in option.php).
Maybe that one also needs a fix?

(I found it because at first my search filter was set to *.php, so I couldn't find setUserSetting.)

I also experienced the not remembering thing, by the way.

comment:3 nacin17 months ago

Yes, the entire API (which spans PHP and JS) will need fixing.

comment:4 cbaldelomar10 months ago

I just came across this bug in my plugin development. The bug occurs on line 147 of the file /wp-includes/js/utils.js

n = name.toString().replace(/[^A-Za-z0-9_]/, ''), v = value.toString().replace(/[^A-Za-z0-9_]/, '');

To fix this problem, we will need to add a hypen to the regular expression list when storing to variable {v}.

n = name.toString().replace(/[^A-Za-z0-9_]/, ''), v = value.toString().replace(/[^A-Za-z0-9_-]/, '');

It is also interesting to note that this javascript replace method will only replace the first occurrence of the string. So if I added an image size with the name "my-custom-image-size", the original line of code would return "mycustom-image-size". I do not think this would have been the developers intended purpose. This can be easily fixed by adding the 'g' flag in the replace function.

n = name.toString().replace(/[^A-Za-z0-9_]/g, ''), v = value.toString().replace(/[^A-Za-z0-9_-]/g, '');

I would be more than happy to submit my very first patch to the Wordpress core. However, I'm not sure if this needs to be discussed more as an acceptable solution. What do you all think?

comment:5 nacin3 months ago

  • Component changed from General to Options and Meta
Note: See TracTickets for help on using tickets.