#37638 closed enhancement (fixed)
Allow plugins to do comprehensive late validation of settings
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Customize | Keywords: | has-patch needs-testing has-dev-note |
Focuses: | Cc: |
Description
With #34893 there was introduced a new pass/fail validation model for Customizer settings. The constructor for WP_Customize_Setting
will add a customize_validate_{$setting->id}
filter if there is a supplied validate_callback
param. Then inside of WP_Customize_Setting::validate()
it applies_filters
on this hook.
There is currently a scenario where a plugin could create a subclass of WP_Customize_Setting
and with a custom validate
method that neglects to apply_filters
. For example:
<?php class Required_Setting extends WP_Customize_Setting { function validate( $value ) { if ( empty( $value ) ) { return new WP_Error( 'empty_value', __( 'You must supply a value' ) ); } return true; } }
The simplest fix here for a plugin would be to explicitly call parent::validate( $value )
like so:
<?php class Required_Setting extends WP_Customize_Setting { function validate( $value ) { if ( empty( $value ) ) { return new WP_Error( 'empty_value', __( 'You must supply a value' ) ); } return parent::validate( $value ); // <= apply_filters on "customize_validate_{$this->id}" } }
Nevertheless, since it is not mandated that a subclass's override method call its parent class's overridden method, there should be some way to guarantee that these filters get applied to account for this edge case.
There are two options I see:
1) We could just (re-)apply the filter in WP_Customize_Manager::validate_setting_values()
:
--- src/wp-includes/class-wp-customize-manager.php +++ src/wp-includes/class-wp-customize-manager.php @@ -1013,6 +1013,10 @@ final class WP_Customize_Manager { } $validity = $setting->validate( $unsanitized_value ); + if ( ! is_wp_error( $validity ) ) { + /** This filter is documented in wp-includes/class-wp-customize-setting.php */ + $validity = apply_filters( "customize_validate_{$setting->id}", $validity, $unsanitized_value, $setting ); + } if ( ! is_wp_error( $validity ) ) { $value = $setting->sanitize( $unsanitized_value ); if ( is_null( $value ) ) { $validity = false;
2) We could introduce a new filter in WP_Customize_Manager::validate_setting_values()
that applies on the array of setting validities:
-
src/wp-includes/class-wp-customize-manager.php
final class WP_Customize_Manager { 1025 1025 } 1026 1026 $validities[ $setting_id ] = $validity; 1027 1027 } 1028 1029 /** 1030 * Filters the result of validating the setting values after each setting's validate method is called. 1031 * 1032 * @see WP_Customize_Setting::validate() 1033 * 1034 * @param array $validities Validities for settings, mapping setting ID to validity. 1035 * @param array $setting_values Unsanitized setting values. 1036 * @param WP_Customize_Manager $wp_customize Manager. 1037 */ 1038 $validities = apply_filters( 'customize_setting_validities', $validities, $setting_values, $this ); 1039 1028 1040 return $validities; 1029 1041 }
Attachments (2)
Change History (14)
#2
follow-up:
↓ 3
@
9 years ago
@chandrapatel I don't really see applying filters a second time to be a big problem. There is a problem with your alternate solution to move the apply_filters
call: this needs to be done inside the validate
method because if filters are used for validation, then if a plugin calls the validate()
method somewhere else then it won't then work. It is intended that the validate()
should be able to be called from other places than just validate_setting_values()
.
#3
in reply to:
↑ 2
@
9 years ago
Replying to westonruter:
There is a problem with your alternate solution to move the
apply_filters
call: this needs to be done inside thevalidate
method because if filters are used for validation, then if a plugin calls thevalidate()
method somewhere else then it won't then work. It is intended that thevalidate()
should be able to be called from other places than justvalidate_setting_values()
.
Oh yes, I got your point.
I don't really see applying filters a second time to be a big problem.
Then Option-1 solution is better instead of introducing new filter. And also developer no need to rely on two different filters.
#4
@
8 years ago
- Keywords needs-dev-note added
It's unlikely that running filters multiple times here will result in a problem, but it's certainly plausible. I would love to see this land early and get called out for testing. Likely can be during the normal customizor post, but I'm also happy to add a note to this week in core when this lands.
This ticket was mentioned in Slack in #core by desrosj. View the logs.
8 years ago
#6
@
8 years ago
- Keywords needs-testing added
As @jorbin stated, there is a small potential for an issue running filters multiple times. This patch needs testing to make sure there are no problems.
This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
8 years ago
#10
@
8 years ago
- Keywords commit added
I'll go with 37638.option1.diff since it doesn't introduce a new filter and any handlers are designed to be run multiple times.
#12
@
5 years ago
- Keywords has-dev-note added; needs-dev-note commit removed
This was detailed in the following dev note: https://make.wordpress.org/core/2016/11/30/customizer-improvements-in-4-7/.
Hello @westonruter
I am seeing the problem with Option-1 solution is possible same filter apply twice from different files if developer didn't create subclass of
WP_Customize_Setting
.In Option-2 solution,
customize_setting_validities
is new filter which also allow developer to validate setting.So developer either do validation by adding filter on
customize_validate_{$setting->id}
orcustomize_setting_validities
.Another solution is add
customize_validate_{$setting->id}
filter insideWP_Customize_Manager::validate_setting_values()
and remove fromWP_Customize_Setting::validate()
, so developer can use only one filter to validate setting and no need to introduce of new filter.