Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#38865 closed defect (bug) (fixed)

Customize: Setting validation error messages can fail to clear if newly-supplied valid value matches previously-valid value stored in changeset

Reported by: westonruter's profile westonruter Owned by: westonruter's profile westonruter
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch
Focuses: Cc:

Description

In #38705 a change was made to the WP_Customize_Manager::save_changeset_post() method to skip validating any incoming dirty setting values that are equal to values that have already been written to the changeset. This was done to prevent changes from one user to be inadvertently overridden by a second user, thus either causing the user_id to be changed or worse for the setting change to be rejected since the second user may not have the required capability. A problem with this change was discovered in which supplying the same valid value was previously supplied (the value which was stored in the changeset) gets skipped and so the validation error never gets cleared.

Consider a setting which allows values 1-4. Given a control for this setting, let the user supply the value 4, and thus 4 is written into the changeset. Now, if a user supplies an invalid value of 5 and then preview then refreshes and communicates the setting_invalidities to the parent window, and the control then gets the error notification. Now, if the user then re-supplies the value of 4 and then immediately clicks into the preview, a changeset update request will happen since the customizer window lost focus. The WP_Customize_Manager::save_changeset_post() method sees that this incoming value is the same as is currently in the snapshot, and so it is skipped, the result of which is that the valid setting flag is not included among the setting_validities in the response, and so the JS response handler fails to clear the error notification. Likewise, since the changeset update request happened immediately, the subsequent full refresh or selective refresh of the preview will not have any dirty settings to preview and so it will simply load the iframe with the natural URL with the customize_changeset_uuid query param (as opposed to doing a form POST into an about:blank preview window), also resulting in an empty setting_validities.

The bug would likely not be experienced frequently by users, but when it does happen, it is very confusing.

Attachments (2)

38865.0.diff (1.6 KB) - added by westonruter 8 years ago.
38865.1.diff (11.3 KB) - added by westonruter 8 years ago.
https://github.com/xwp/wordpress-develop/pull/202

Download all attachments as: .zip

Change History (5)

@westonruter
8 years ago

#1 @westonruter
8 years ago

  • Keywords has-patch added

#2 @westonruter
8 years ago

  • Owner set to westonruter
  • Status changed from new to accepted

#3 @westonruter
8 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 39320:

Customize: Ensure that WP_Customize_Manager::save_changeset_post() returns setting_validities even for supplied values that are unchanged from values in changeset.

Check setting existence and authorization via WP_Customize_Manager::validate_setting_values() even for null values to account for custom params being added to settings, preventing failures from being silently ignored.

See #38705, #30937.
Fixes #38865.

Note: See TracTickets for help on using tickets.