WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 22 months ago

#34893 closed enhancement

Improve Customizer setting validation model — at Version 10

Reported by: westonruter Owned by: westonruter
Milestone: 4.6 Priority: high
Severity: normal Version: 3.4
Component: Customize Keywords: has-patch has-screenshots has-unit-tests commit
Focuses: javascript Cc:

Description (last modified by westonruter)

Customizer setting values need to be able to be validated and for any validation errors to block the Customizer from saving any setting until all are valid.

Settings in the Customizer rely on sanitization to ensure that only valid values get persisted to the database. The sanitization in the Customizer generally allows values to be passed through to be persisted and does not enforce validation that blocks the saving of the value. This is in large part because there is no standard facility for showing error messages relating to Customizer controls, and there is no standard method to raise validation errors. A Customizer setting can be blocked from being saved by returning null from the WP_Customize_Setting::sanitize() method (i.e. generally returned via customize_sanitize_{$setting_id}). When this is done, however, the modified value completely disappears from the preview with no indication for why the value seems to be reset to the default.

In JavaScript there is the wp.customize.Setting.prototype.validate() method that can be extended to return null in the case where the value should be rejected, but again this does not provide a way to display a validation message: the user can be entering data but they just stop seeing the value making changes in the preview without any feedback. Even worse, if the JS validate method actually manipulates the value to make it valid, there can be strange behavior in the UI as the user provides input, likely having to do with the two-way data binding of wp.customize.Element instances.

Furthermore, if one setting is blocked from being saved by means of validation in the sanitization method, the other settings in the Customizer state may very well be completely valid, and thus they would save successfully. The result is that some settings would get saved, whereas others would not, and the user wouldn't know which were successful and which failed (again, since there is no standard mechanism for showing validation error message). The Customizer state would only partially get persisted to the database. This isn't good.

We need to improve the Customizer by:

  • Validating settings on server before save.
  • Displaying validation error messages from server and from JS client.
  • Performing transactional/atomic setting saving, rejecting all settings if one is invalid.

Since the WP_Customize_Setting::sanitize() already partially supports validation by returning null, we can build on that to also allow it to return a WP_Error object. We'd want to retain the regular fuzzy sanitization during preview updates, but during the customize_save action we'd want to impose a strict pass/fail for setting validation.

This is closely related and complimentary to #30937: Add Customizer transactions

For the notification bar to display validation error messages, see #35210.

Change History (15)

#1 @westonruter
4 years ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Milestone changed from Future Release to 4.5
  • Owner set to westonruter
  • Status changed from new to accepted

I have implemented this in a feature plugin called “Customize Setting Validation”.

To do server-side validation of a setting, implement a setting sanitizer that returns null or a WP_Error object:

<?php
$wp_customize->add_setting( 'year_established', array(
        'type' => 'option',
        'sanitize_callback' => function ( $value ) {
                $value = intval( $value );

                // Apply strict validation when the sanitize callback is called during.customize_validate_settings.
                if ( doing_action( 'customize_validate_settings' ) && ( $value < 1900 || $value > 2100 ) ) {
                        return new WP_Error( 'invalid_value', __( 'Year must be between 1900 and 2100.' ) );
                }

                $value = min( 2100, max( $value, 1900 ) );
                return $value;
        }
) );

By returning null, the Customizer will display a generic error message. By returning WP_Error, the specific message can be provided.

The validation error message can also be set programmatically by JS by calling control.validationMessage.set(),
for example from an extended control.setting.validate() method. The validationMessage is inspired by HTML5.

For a demonstration of the functionality made possible with this Customizer setting validation API,
including how to do client-side validation, see the “Customize_Validate_Entitled_Settings” plugin. It will validate that the Site Name (blogname), nav menu item titles, and widget titles are all fully populated. The validation is done both on the client and on the server.

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


4 years ago

#3 @westonruter
4 years ago

  • Description modified (diff)

@westonruter
4 years ago

POC

#4 @westonruter
4 years ago

Instead of overloading the Customizer setting's sanitize_callback to do both sanitization and validation, for a Core patch the WP_Customizer_Setting should really also implement a validate_callback, just as has been done for the REST API (see WP_REST_Request::has_valid_params()). There can be a default validate_callback that checks if the sanitize_callback returns null and if so, then it can return false. Otherwise, it can return true. The REST API also supports the validate_callback returning a WP_Error, so our validate_callback in WP_Customize_Setting can do the same, something like 34893.0.diff.

Note the addition of a new $strict argument to the customize_sanitize filter which allows the filter to do strict/validation checking. I'm not 100% sold on this, but it would be nice if there could be some way to more easily integrate the sanitization/validation logic.

Maybe the REST API folks have some insights here :). The goal here is to keep the Customizer API as close to the REST API, so that Customizer settings can be just added to the REST API and it will just work.

/cc @rmccue @rachelbaker @danielbachhuber @joehoyle

#5 follow-up: @joehoyle
4 years ago

I agree on wanting to split the validate / sanitization. Validate can return true / false and optionally a WP_Error to provide extra validation feedback to the user. sanitize is used to mutate the value to the correct datatype, be it a safe string, bool, int etc.

I'd imagine you could just call the validate_callback in isolation also, for example, checking an inputs validity on keyup / blur.

@rmccue and myself have discussed for the REST API on Save, whether the validate callback should be called on the value pre or post sanitize. I think there's merit to both, depending on the use case, we never came to a hard decision on that, but core currently calls sanitize before validate. This means the validator function can deal with "clean" data, and is concerned with specific validity rather than type checks. For this way round, the example of posts_per_page is a good one: sanitize_callback calls absint on the data passed via POST, and the validate_callback would then > 0 && < 100.

@westonruter
4 years ago

Invalid site title

@westonruter
4 years ago

Invalid widget title

@westonruter
4 years ago

Invalid nav menu item title

#6 in reply to: ↑ 5 @westonruter
4 years ago

Replying to joehoyle:

@rmccue and myself have discussed for the REST API on Save, whether the validate callback should be called on the value pre or post sanitize. I think there's merit to both, depending on the use case, we never came to a hard decision on that, but core currently calls sanitize before validate. This means the validator function can deal with "clean" data, and is concerned with specific validity rather than type checks. For this way round, the example of posts_per_page is a good one: sanitize_callback calls absint on the data passed via POST, and the validate_callback would then > 0 && < 100.

What about if the original raw unsanitized value is passed into the validator along with the sanitized value? This would allow the validator to have the flexibility to determine whether or not the sanitized value can be considered “good enough” for persisting to the DB, or else it can do a strict check to fail it simply if $sanitized_value !== $unsanitized_value.

In that regard, the patch for this ticket could be modified to pass in both the $sanitized_value and the $unsanitized_value to the customize_validate_{$setting_id} filter. I've amended my patch to implement that: 34893.1.diff. It also adds:

  • Adds a $strict second parameter to WP_Customize_Setting::save(), which is a signal that WP_Error can be returned
  • Validate all settings before the customize_save action, and return with a JSON error with an array of all the invalid settings if any are invalid.

For the REST API, however, it doesn't look like the original/raw/unsanitized value would still be available for the validate callback to look at?

Last edited 4 years ago by westonruter (previous) (diff)

#7 @sc0ttkclark
4 years ago

I'm keeping an eye on this for use in the Fields API, I could see this as super useful there as well, albeit the usage would be different outside of the Customizer -- I'd love to see what it could allow for.

#8 @westonruter
4 years ago

Another improvement is needed on the JS side of things. Namely, there is currently a wp.customize.Setting.prototype.validate() method allows you to sanitize a value and also to block the setting update if you pass back null to block the Setting (really, the Value). So really, the validate method in JS is serving in the same purpose as the sanitize method in PHP. They both conflate sanitization and validation.

I suggest that we extend wp.customize.Setting to introduce a sanitize method for the purpose of sanitization, and that the existing validate method be extended to allow for the returning of an Error object, which would have the same effect for blocking the value but it could also then emit an event on the Setting object, which could then be picked up by any controls to then display a validation error message.

Last edited 4 years ago by westonruter (previous) (diff)

#9 @westonruter
4 years ago

See demo video of the feature plugin: https://www.youtube.com/watch?v=ZNk6FhtS8TM

#10 @westonruter
4 years ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.