Commit 73568a4aec4 for woocommerce
commit 73568a4aec4173250c53b405ca70b2ba244fe86e
Author: Daniel Mallory <daniel.mallory@automattic.com>
Date: Thu Jul 23 23:05:58 2026 +0100
Fix boolean and visibility canonicalization in Settings UI schemas (#66936)
* fix(settings): Convert booleans like the client String() coercion
The Settings UI client matches select options against values with JS
String() coercion, where true is 'true' and false is 'false'. Schema
canonicalization used the PHP string cast, which turns true into '1'
and false into '', so a boolean field value that matched a 'true' or
'false' string option before conversion stopped matching after it, and
false collapsed to the empty no-selection value.
Cast scalars through a helper that mirrors String() semantics so
canonicalizing a value never changes which option it matches. Integer
and float casts are unaffected; PHP and JS agree on their string forms.
Refs #66934
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(settings): Canonicalize visibility values with their controller
Canonicalization stringifies the value of a field that carries options,
but left the visibility rules of dependent fields untouched. The client
compares the controller value against the rule value with strict
equality, so a numeric rule pointing at an options controller stopped
matching once the controller value became a string, and the dependent
field silently flipped visibility.
Convert rule values the same way when their controller carries options,
and name the converted fields in the doing-it-wrong notice. Rules on
controllers without options, like checkboxes, pass through unchanged
because their runtime values stay booleans.
Refs #66934
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(settings): Correct the canonicalization version to 11.0.0
Schema canonicalization lands in WooCommerce 11.0.0 through the
backport of PR #66906 to release/11.0, so the @since annotation and
the doing-it-wrong notice version pointed one release too late.
Refs #66934
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(settings): Correct two more Settings UI since tags to 11.0.0
A sweep of release/11.0 for stray 11.1.0 strings found two more
annotations on backported Settings UI code: get_schema() in
RegisteredSettingsSectionAdapter and is_drill_down() in
SettingsUIRequestContext. Both ship in 11.0.0.
Refs #66934
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(settings): Convert floats locale-independently in canonicalization
A plain float-to-string cast follows LC_NUMERIC before PHP 8.0, so a
comma-decimal locale turns 1.5 into '1,5', a string the client String()
coercion can never produce. Convert floats through wc_float_to_string()
so the decimal separator is always a dot.
Refs #66935
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* refactor(settings): Name the visibility rule guard
The seven-condition guard in the visibility pass was hard to read.
Split it into the same field-shape check the first pass uses plus an
is_canonicalizable_visibility_rule() predicate stating the intent: the
rule carries a value and compares against an options field.
Refs #66934
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(settings): Cover non-scalar members in visibility value lists
canonicalize_scalar_list() returns null when a visibility value list
contains a non-scalar member, and the visibility pass only overwrites
the list on a non-null result. That guard had no test, so a regression
would go unnoticed. Add a case asserting such a list passes through
whole, scalar members included, for the provider to fix.
Raised in review on PR #66936.
Refs #66934
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
diff --git a/plugins/woocommerce/changelog/fix-66934-settings-ui-boolean-canonicalization b/plugins/woocommerce/changelog/fix-66934-settings-ui-boolean-canonicalization
new file mode 100644
index 00000000000..2e05d939463
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-66934-settings-ui-boolean-canonicalization
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Convert boolean Settings UI schema values the way the client does and canonicalize visibility values alongside their controller field.
diff --git a/plugins/woocommerce/src/Internal/Admin/Settings/RegisteredSettingsSectionAdapter.php b/plugins/woocommerce/src/Internal/Admin/Settings/RegisteredSettingsSectionAdapter.php
index 55d6067406d..6db4f04fac5 100644
--- a/plugins/woocommerce/src/Internal/Admin/Settings/RegisteredSettingsSectionAdapter.php
+++ b/plugins/woocommerce/src/Internal/Admin/Settings/RegisteredSettingsSectionAdapter.php
@@ -47,7 +47,7 @@ class RegisteredSettingsSectionAdapter extends LegacySettingsPageAdapter {
* @param string $section Unused. This adapter wraps a single registered section.
* @return array
*
- * @since 11.1.0
+ * @since 11.0.0
*/
public function get_schema( string $section ): array {
$schema = parent::get_schema( $section );
diff --git a/plugins/woocommerce/src/Internal/Admin/Settings/SettingsUIRequestContext.php b/plugins/woocommerce/src/Internal/Admin/Settings/SettingsUIRequestContext.php
index 8406b8d3f24..87506524993 100644
--- a/plugins/woocommerce/src/Internal/Admin/Settings/SettingsUIRequestContext.php
+++ b/plugins/woocommerce/src/Internal/Admin/Settings/SettingsUIRequestContext.php
@@ -273,7 +273,7 @@ class SettingsUIRequestContext {
* settings tabs. Pages registered at the top level of settings are not
* drill-downs: they hide the header and keep the tabs.
*
- * @since 11.1.0
+ * @since 11.0.0
*
* @return bool
*/
diff --git a/plugins/woocommerce/src/Internal/Admin/Settings/SettingsUISchema.php b/plugins/woocommerce/src/Internal/Admin/Settings/SettingsUISchema.php
index 9676d9ddb65..2675a4d886b 100644
--- a/plugins/woocommerce/src/Internal/Admin/Settings/SettingsUISchema.php
+++ b/plugins/woocommerce/src/Internal/Admin/Settings/SettingsUISchema.php
@@ -143,11 +143,12 @@ class SettingsUISchema {
*
* Schemas built from legacy settings always carry string option values, but
* native providers can supply any scalar. The client matches options against
- * the stored value with strict string comparison, so scalar option values
- * and the selected values they match are cast to strings here. Malformed
- * entries remain unchanged for the provider to fix.
+ * the stored value with strict string comparison, so scalar option values,
+ * the selected values they match, and visibility values compared against
+ * them are cast here to the string the client's own String() coercion
+ * produces. Malformed entries remain unchanged for the provider to fix.
*
- * @since 11.1.0
+ * @since 11.0.0
*
* @param array $schema Settings UI schema.
* @return array Schema with scalar option values canonicalized to strings.
@@ -158,6 +159,7 @@ class SettingsUISchema {
}
$converted_fields = array();
+ $option_field_ids = array();
foreach ( $schema['groups'] as &$group ) {
if ( ! is_array( $group ) || ! isset( $group['fields'] ) || ! is_array( $group['fields'] ) ) {
@@ -174,7 +176,8 @@ class SettingsUISchema {
continue;
}
- $converted = false;
+ $option_field_ids[] = $field['id'];
+ $converted = false;
foreach ( $field['options'] as &$option ) {
if (
@@ -186,14 +189,14 @@ class SettingsUISchema {
continue;
}
- $option['value'] = (string) $option['value'];
+ $option['value'] = self::to_canonical_string( $option['value'] );
$converted = true;
}
unset( $option );
if ( array_key_exists( 'value', $field ) ) {
if ( is_scalar( $field['value'] ) && ! is_string( $field['value'] ) ) {
- $field['value'] = (string) $field['value'];
+ $field['value'] = self::to_canonical_string( $field['value'] );
$converted = true;
} elseif ( is_array( $field['value'] ) ) {
$canonical_list = self::canonicalize_scalar_list( $field['value'] );
@@ -212,15 +215,46 @@ class SettingsUISchema {
}
unset( $group );
+ foreach ( $schema['groups'] as &$group ) {
+ if ( ! is_array( $group ) || ! isset( $group['fields'] ) || ! is_array( $group['fields'] ) ) {
+ continue;
+ }
+
+ foreach ( $group['fields'] as &$field ) {
+ if ( ! is_array( $field ) || ! isset( $field['id'] ) || ! is_string( $field['id'] ) ) {
+ continue;
+ }
+
+ if ( ! self::is_canonicalizable_visibility_rule( $field['visibility'] ?? null, $option_field_ids ) ) {
+ continue;
+ }
+
+ $rule_value = $field['visibility']['value'];
+
+ if ( is_scalar( $rule_value ) && ! is_string( $rule_value ) ) {
+ $field['visibility']['value'] = self::to_canonical_string( $rule_value );
+ $converted_fields[] = $field['id'];
+ } elseif ( is_array( $rule_value ) ) {
+ $canonical_list = self::canonicalize_scalar_list( $rule_value );
+ if ( null !== $canonical_list ) {
+ $field['visibility']['value'] = $canonical_list;
+ $converted_fields[] = $field['id'];
+ }
+ }
+ }
+ unset( $field );
+ }
+ unset( $group );
+
if ( ! empty( $converted_fields ) ) {
wc_doing_it_wrong(
__METHOD__,
sprintf(
/* translators: %s: comma-separated field ids. */
- esc_html__( 'A Settings UI schema provider supplied non-string option or field values that WooCommerce converted for compatibility: %s. Update the provider to supply string values.', 'woocommerce' ),
+ esc_html__( 'A Settings UI schema provider supplied non-string option, field, or visibility values that WooCommerce converted for compatibility: %s. Update the provider to supply string values.', 'woocommerce' ),
esc_html( implode( ', ', array_unique( $converted_fields ) ) )
),
- '11.1.0'
+ '11.0.0'
);
}
@@ -250,7 +284,43 @@ class SettingsUISchema {
}
}
- return $needs_conversion ? array_map( 'strval', $values ) : null;
+ return $needs_conversion ? array_map( array( __CLASS__, 'to_canonical_string' ), $values ) : null;
+ }
+
+ /**
+ * Whether a visibility rule carries a value compared against an options field.
+ *
+ * @param mixed $rule Candidate visibility rule.
+ * @param array $option_field_ids Ids of fields carrying an options array.
+ * @return bool
+ */
+ private static function is_canonicalizable_visibility_rule( $rule, array $option_field_ids ): bool {
+ return is_array( $rule )
+ && array_key_exists( 'value', $rule )
+ && in_array( $rule['controller'] ?? null, $option_field_ids, true );
+ }
+
+ /**
+ * Cast a scalar to the string the client's String() coercion produces, so
+ * canonicalizing a value never changes which option or visibility rule it
+ * matches. PHP casts diverge from String() for booleans: (string) true is
+ * '1' and (string) false is '', while String() gives 'true' and 'false'.
+ * Floats convert through wc_float_to_string() because a plain cast is
+ * locale-sensitive before PHP 8.0 and String() never emits a comma.
+ *
+ * @param bool|int|float|string $value Scalar value.
+ * @return string
+ */
+ private static function to_canonical_string( $value ): string {
+ if ( is_bool( $value ) ) {
+ return $value ? 'true' : 'false';
+ }
+
+ if ( is_float( $value ) ) {
+ return wc_float_to_string( $value );
+ }
+
+ return (string) $value;
}
/**
diff --git a/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/SettingsUISchemaTest.php b/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/SettingsUISchemaTest.php
index 61b58cc91b3..0084a19bfd1 100644
--- a/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/SettingsUISchemaTest.php
+++ b/plugins/woocommerce/tests/php/src/Internal/Admin/Settings/SettingsUISchemaTest.php
@@ -426,11 +426,54 @@ class SettingsUISchemaTest extends WC_Unit_Test_Case {
$field = $schema['groups']['main']['fields'][0];
$this->assertSame( '1', $field['value'] );
- $this->assertSame( array( '1', '1' ), array_column( $field['options'], 'value' ), 'Boolean option values should use the PHP string cast, matching stored values.' );
+ $this->assertSame( array( '1', 'true' ), array_column( $field['options'], 'value' ), 'Boolean option values should convert like the client String() coercion, not the PHP string cast.' );
}
/**
- * @testdox It canonicalizes float option values with the PHP string cast.
+ * @testdox It canonicalizes boolean values to the strings the client String() coercion produced before conversion.
+ */
+ public function test_canonicalize_option_values_matches_client_coercion_for_booleans(): void {
+ $this->setExpectedIncorrectUsage( SettingsUISchema::class . '::canonicalize_option_values' );
+
+ $string_options = array(
+ array(
+ 'label' => 'On',
+ 'value' => 'true',
+ ),
+ array(
+ 'label' => 'Off',
+ 'value' => 'false',
+ ),
+ );
+
+ $schema = SettingsUISchema::canonicalize_option_values(
+ $this->get_native_schema_with_fields(
+ array(
+ array(
+ 'id' => 'acme_enabled',
+ 'type' => 'select',
+ 'value' => true,
+ 'options' => $string_options,
+ ),
+ array(
+ 'id' => 'acme_disabled',
+ 'type' => 'select',
+ 'value' => false,
+ 'options' => $string_options,
+ ),
+ )
+ )
+ );
+
+ $fields = $schema['groups']['main']['fields'];
+
+ $this->assertSame( 'true', $fields[0]['value'], 'A true value must keep matching the string option the client matched before canonicalization.' );
+ $this->assertSame( 'false', $fields[1]['value'], 'A false value must not collapse to the empty no-selection string.' );
+ $this->assertSame( array( 'true', 'false' ), array_column( $fields[0]['options'], 'value' ), 'String option values should pass through unchanged.' );
+ }
+
+ /**
+ * @testdox It canonicalizes float option values locale-independently.
*/
public function test_canonicalize_option_values_stringifies_float_values(): void {
$this->setExpectedIncorrectUsage( SettingsUISchema::class . '::canonicalize_option_values' );
@@ -458,7 +501,7 @@ class SettingsUISchemaTest extends WC_Unit_Test_Case {
$field = $schema['groups']['main']['fields'][0];
$this->assertSame( '1.5', $field['value'] );
- $this->assertSame( array( '0.5', '1.5' ), array_column( $field['options'], 'value' ), 'Float option values should use the PHP string cast, matching stored values.' );
+ $this->assertSame( array( '0.5', '1.5' ), array_column( $field['options'], 'value' ), 'Float option values should convert with a dot decimal separator in any locale, matching the client String() coercion.' );
}
/**
@@ -536,6 +579,116 @@ class SettingsUISchemaTest extends WC_Unit_Test_Case {
$this->assertEquals( $schema, SettingsUISchema::canonicalize_option_values( $schema ), 'Malformed entries should pass through for the provider to fix.' );
}
+ /**
+ * @testdox It canonicalizes visibility values compared against an options field.
+ */
+ public function test_canonicalize_option_values_stringifies_visibility_values_for_option_controllers(): void {
+ $this->setExpectedIncorrectUsage( SettingsUISchema::class . '::canonicalize_option_values' );
+
+ $schema = SettingsUISchema::canonicalize_option_values(
+ $this->get_native_schema_with_fields(
+ array(
+ array(
+ 'id' => 'acme_tier',
+ 'type' => 'select',
+ 'value' => 1,
+ 'options' => array(
+ array(
+ 'label' => 'One',
+ 'value' => 1,
+ ),
+ array(
+ 'label' => 'Two',
+ 'value' => 2,
+ ),
+ ),
+ ),
+ array(
+ 'id' => 'acme_tier_notes',
+ 'type' => 'text',
+ 'value' => '',
+ 'visibility' => array(
+ 'controller' => 'acme_tier',
+ 'value' => 1,
+ ),
+ ),
+ array(
+ 'id' => 'acme_tier_badge',
+ 'type' => 'text',
+ 'value' => '',
+ 'visibility' => array(
+ 'controller' => 'acme_tier',
+ 'value' => array( 1, true ),
+ ),
+ ),
+ )
+ )
+ );
+
+ $fields = $schema['groups']['main']['fields'];
+
+ $this->assertSame( '1', $fields[1]['visibility']['value'], 'Scalar visibility values must convert with the controller value they are compared against.' );
+ $this->assertSame( array( '1', 'true' ), $fields[2]['visibility']['value'], 'Visibility value lists must convert with the controller value they are compared against.' );
+ }
+
+ /**
+ * @testdox It leaves visibility value lists containing a non-scalar member unchanged.
+ */
+ public function test_canonicalize_option_values_leaves_visibility_value_lists_with_non_scalar_members_unchanged(): void {
+ $schema = $this->get_native_schema_with_fields(
+ array(
+ array(
+ 'id' => 'acme_tier',
+ 'type' => 'select',
+ 'value' => '1',
+ 'options' => array(
+ array(
+ 'label' => 'One',
+ 'value' => '1',
+ ),
+ ),
+ ),
+ array(
+ 'id' => 'acme_tier_notes',
+ 'type' => 'text',
+ 'value' => '',
+ 'visibility' => array(
+ 'controller' => 'acme_tier',
+ 'value' => array( 1, array( 'not-scalar' ) ),
+ ),
+ ),
+ )
+ );
+
+ $this->assertSame( $schema, SettingsUISchema::canonicalize_option_values( $schema ), 'Visibility value lists with a non-scalar member should pass through whole, scalar members included, for the provider to fix.' );
+ }
+
+ /**
+ * @testdox It leaves visibility values unchanged when the controller has no options.
+ */
+ public function test_canonicalize_option_values_leaves_visibility_values_for_non_option_controllers(): void {
+ $schema = $this->get_native_schema_with_fields(
+ array(
+ array(
+ 'id' => 'acme_enabled',
+ 'type' => 'checkbox',
+ 'value' => true,
+ ),
+ array(
+ 'id' => 'acme_enabled_notes',
+ 'type' => 'text',
+ 'value' => '',
+ 'visibility' => array(
+ 'controller' => 'acme_enabled',
+ 'value' => true,
+ ),
+ ),
+ )
+ );
+
+ $this->assertSame( $schema, SettingsUISchema::canonicalize_option_values( $schema ), 'Checkbox controllers compare boolean values, so their visibility rules should pass through unchanged.' );
+ }
+
/**
* @testdox It leaves associative value arrays unchanged.
*/
@@ -564,13 +717,23 @@ class SettingsUISchemaTest extends WC_Unit_Test_Case {
* @return array
*/
private function get_native_schema_with_field( array $field ): array {
+ return $this->get_native_schema_with_fields( array( $field ) );
+ }
+
+ /**
+ * Build a minimal native schema with the given fields.
+ *
+ * @param array $fields Field definitions.
+ * @return array
+ */
+ private function get_native_schema_with_fields( array $fields ): array {
return array(
'id' => 'acme',
'title' => 'Acme',
'groups' => array(
'main' => array(
'id' => 'main',
- 'fields' => array( $field ),
+ 'fields' => $fields,
),
),
);