WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#37010 closed enhancement (wontfix)

Remove early exits from `WP_Customize_Setting::validate()` on `WP_Error` or `null`

Reported by: dlh Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.6
Component: Customize Keywords:
Focuses: Cc:

Description

Currently, WP_Customize_Setting::validate() returns immediately if WP_Error or null is passed to it.

I wonder whether validate() could continue to the end of the method even for those values, including the "customize_validate_{$setting_id}" filter.

Two reasons for this change would be:

  1. Currently, any WP_Error passed to validate() is assumed to have errors -- it isn't subject to the empty() check at the end of the method.
  1. It might be nice to be able to return null from WP_Customize_Setting::sanitize() for backwards compatibility but still add details about the error with the 'customize_validate_' filter where it's supported.

Patch attached with one approach to the change.

Attachments (1)

37010.diff (1.2 KB) - added by dlh 5 years ago.

Download all attachments as: .zip

Change History (6)

@dlh
5 years ago

#1 @westonruter
5 years ago

@dlh Thanks for raising this. The reason why WP_Customize_Setting::validate() is short-circuiting is because if WP_Error is passed in, then we basically have no additional context for what we could amend to this WP_Error instance. In other words, the setting $value wasn't passed in, so we can't look at it to do any validation. Right?

#2 @dlh
5 years ago

Yeah. I'm not so sure now about point 2. either, as compared with using 'customize_save_validation_before' to adjust the sanitize() behavior.

#3 @westonruter
5 years ago

This is closely related to #37247: namely once that lands, it will be basically not be possible (in core) for a WP_Error to be passed into the validate callback since the sanitize callback will be called after the validate callback. @dlh with that landed, is this ticket still relevant?

#4 @dlh
5 years ago

Nope, I don't think it's relevant.

#5 @westonruter
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.