Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#18914 closed defect (bug) (fixed)

register_setting() sanitisation callback two argument backwards incompatibility

Reported by: duck_'s profile duck_ Owned by: duck_'s profile duck_
Milestone: 3.3 Priority: high
Severity: major Version: 3.3
Component: General Keywords: has-patch
Focuses: Cc:

Description

See #15335 and [18887] for the introduction of always passing two arguments to the sanitisation callback. This is a problem because it breaks plugins which use register_setting with a sanitisation callback which chokes on the 2nd argument being introduced. An example of this is provided on the codex:

register_setting( 'my_options_group', 'my_option_name', 'intval' );

One plugin which uses this is bbPress. I was able trigger this on activation on a fresh install:

Warning: intval() expects parameter 2 to be long, string given in wp-includes/plugin.php on line 170
call_user_func_array('intval', array (0 => '5', 1 => '_bbp_edit_lock')) wp-includes/plugin.php:170

Attached is a patch to revert the change.

The only issue is how to address the reason for the code change: "You should be able to have a generic save handler that does a switch on the option name and does your sanitization magic." A generic sanitize_option filter would do it, but would incur some overhead on having the "generic save handler" called for every option.

Attachments (2)

revert-r18887.diff (479 bytes) - added by duck_ 12 years ago.
plugin.php.diff (1.2 KB) - added by sirzooro 12 years ago.
register_setting() with extra param

Download all attachments as: .zip

Change History (6)

@duck_
12 years ago

#1 @sirzooro
12 years ago

  • Cc sirzooro added

I propose another approach - add extra param to register_setting(), so user would be able to decide if it wants to get option name or not. I have implemented this in attached patch.

@sirzooro
12 years ago

register_setting() with extra param

#2 @westi
12 years ago

I think for now we should just revert.

A generic save handler which a switch in it isn't significantly better than a function per option - personally I think the function per option is better.

#3 @nacin
12 years ago

Reminds me of #14671. Per IRC, revert, re-open #15335, punt.

#4 @duck_
12 years ago

  • Owner set to duck_
  • Resolution set to fixed
  • Status changed from new to closed

In [18951]:

Revert r18887 due to back compat issues. Fixes #18914.

For example 'intval' couldn't be used as a sanitisation callback.

Note: See TracTickets for help on using tickets.