From ec94d76c3f2968a0305a599d4968574094ae184b Mon Sep 17 00:00:00 2001 From: "vivi-the-going-merry[bot]" <308115520+vivi-the-going-merry[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:49:43 -0600 Subject: [PATCH 1/3] Give email/url/phone/number fields their own invalid-message copy All field-format validation errors fell back to the same generic "[field name] is invalid" message regardless of field type, giving no correction guidance (WCAG 3.3.1/3.3.3). default_invalid_msg() now takes the field and returns type-specific corrective copy for email, url, phone, and number fields; every other type keeps the existing generic message. get_error_msg()'s own runtime fallback -- the actual path hit for any field with no custom invalid message saved -- now calls into default_invalid_msg() instead of duplicating the generic literal, so the fix reaches real form submissions, not just the builder default. Closes Strategy11/formidable-pro#6749 Co-Authored-By: Claude Sonnet 5 --- classes/helpers/FrmFieldsHelper.php | 50 ++++++++++++++++--- tests/phpunit/fields/test_FrmFieldsHelper.php | 49 ++++++++++++++++++ 2 files changed, 93 insertions(+), 6 deletions(-) diff --git a/classes/helpers/FrmFieldsHelper.php b/classes/helpers/FrmFieldsHelper.php index 9acd4a3995..36558f50bf 100644 --- a/classes/helpers/FrmFieldsHelper.php +++ b/classes/helpers/FrmFieldsHelper.php @@ -293,7 +293,7 @@ private static function fill_cleared_strings( $field, array &$field_array ) { $frm_settings = FrmAppHelper::get_settings(); $field_array['invalid'] = $frm_settings->re_msg; } else { - $field_array['invalid'] = self::default_invalid_msg(); + $field_array['invalid'] = self::default_invalid_msg( $field ); } } @@ -304,13 +304,52 @@ private static function fill_cleared_strings( $field, array &$field_array ) { } /** + * Default "invalid" validation message. Gives field-type-specific correction guidance + * for field types where the format requirement isn't obvious from the label alone + * (WCAG 3.3.1/3.3.3), and falls back to a generic message for every other type. + * * @since 6.8.3 + * @since 6.35 Added the $field param for a type-specific message. + * + * @param array|object|null $field Optional. Field to check the type of. * * @return string */ - public static function default_invalid_msg() { - /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ - return sprintf( __( '%s is invalid', 'formidable' ), '[field_name]' ); + public static function default_invalid_msg( $field = null ) { + $type = ''; + if ( is_array( $field ) ) { + $type = $field['type'] ?? ''; + } elseif ( is_object( $field ) ) { + $type = $field->type ?? ''; + } + + switch ( $type ) { + case 'email': + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + $message = __( '%s is invalid. Enter a valid email address, like name@example.com', 'formidable' ); + break; + + case 'url': + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + $message = __( '%s is invalid. Enter a valid web address, like https://example.com', 'formidable' ); + break; + + case 'phone': + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + $message = __( '%s is invalid. Enter a valid phone number', 'formidable' ); + break; + + case 'number': + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + $message = __( '%s is invalid. Enter a number', 'formidable' ); + break; + + default: + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + $message = __( '%s is invalid', 'formidable' ); + } + + return sprintf( $message, '[field_name]' ); } /** @@ -465,8 +504,7 @@ public static function get_error_msg( $field, $error ) { ), 'invalid' => array( 'full' => __( 'This field is invalid', 'formidable' ), - /* translators: %s: Field name */ - 'part' => sprintf( __( '%s is invalid', 'formidable' ), '[field_name]' ), + 'part' => self::default_invalid_msg( $field ), ), 'blank' => array( 'full' => $frm_settings->blank_msg, diff --git a/tests/phpunit/fields/test_FrmFieldsHelper.php b/tests/phpunit/fields/test_FrmFieldsHelper.php index 741d3dbf14..465ee60a9a 100644 --- a/tests/phpunit/fields/test_FrmFieldsHelper.php +++ b/tests/phpunit/fields/test_FrmFieldsHelper.php @@ -310,4 +310,53 @@ public function test_get_error_msg() { $error_message = FrmFieldsHelper::get_error_msg( $field, 'unique_msg' ); $this->assertSame( 'My example field must be unique', $error_message ); } + + /** + * @covers FrmFieldsHelper::get_error_msg + * @covers FrmFieldsHelper::default_invalid_msg + */ + public function test_get_error_msg_invalid_is_field_type_specific() { + $form_id = $this->factory->form->create(); + + // Email and number fields get their own corrective message when no custom one is set. + $email_field = $this->factory->field->create_and_get( + array( + 'name' => 'Email', + 'form_id' => $form_id, + 'type' => 'email', + ) + ); + + $error_message = FrmFieldsHelper::get_error_msg( $email_field, 'invalid' ); + $this->assertSame( 'Email is invalid. Enter a valid email address, like name@example.com', $error_message ); + + $number_field = $this->factory->field->create_and_get( + array( + 'name' => 'Age', + 'form_id' => $form_id, + 'type' => 'number', + ) + ); + + $error_message = FrmFieldsHelper::get_error_msg( $number_field, 'invalid' ); + $this->assertSame( 'Age is invalid. Enter a number', $error_message ); + + // Field types with no specific copy keep the original generic message. + $text_field = $this->factory->field->create_and_get( + array( + 'name' => 'Comment', + 'form_id' => $form_id, + 'type' => 'text', + ) + ); + + $error_message = FrmFieldsHelper::get_error_msg( $text_field, 'invalid' ); + $this->assertSame( 'Comment is invalid', $error_message ); + + // A custom message saved on the field is never overridden by the type-specific default. + $text_field->field_options['invalid'] = 'Please fix [field_name]'; + + $error_message = FrmFieldsHelper::get_error_msg( $text_field, 'invalid' ); + $this->assertSame( 'Please fix Comment', $error_message ); + } } From 592462560abc045276f5779456b25d25d5262c81 Mon Sep 17 00:00:00 2001 From: "vivi-the-going-merry[bot]" <308115520+vivi-the-going-merry[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2026 19:55:19 -0600 Subject: [PATCH 2/3] Simplify: reuse FrmField::get_field_type(), lookup array over switch, loop test Self-review pass: reuse the existing FrmField::get_field_type() helper for the array/object type extraction instead of reimplementing it, replace the switch with a type => message lookup array (matches the existing FrmXMLHelper.php per-type sprintf(__()) array convention in this codebase), and collapse the three near-identical test blocks into one loop over tuples (matching this test file's own existing convention, e.g. test_value_meets_condition). Co-Authored-By: Claude Sonnet 5 --- classes/helpers/FrmFieldsHelper.php | 46 +++++--------- tests/phpunit/fields/test_FrmFieldsHelper.php | 60 +++++++++---------- 2 files changed, 44 insertions(+), 62 deletions(-) diff --git a/classes/helpers/FrmFieldsHelper.php b/classes/helpers/FrmFieldsHelper.php index 36558f50bf..31097ede22 100644 --- a/classes/helpers/FrmFieldsHelper.php +++ b/classes/helpers/FrmFieldsHelper.php @@ -316,40 +316,24 @@ private static function fill_cleared_strings( $field, array &$field_array ) { * @return string */ public static function default_invalid_msg( $field = null ) { - $type = ''; - if ( is_array( $field ) ) { - $type = $field['type'] ?? ''; - } elseif ( is_object( $field ) ) { - $type = $field->type ?? ''; - } - - switch ( $type ) { - case 'email': - /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ - $message = __( '%s is invalid. Enter a valid email address, like name@example.com', 'formidable' ); - break; - - case 'url': - /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ - $message = __( '%s is invalid. Enter a valid web address, like https://example.com', 'formidable' ); - break; - - case 'phone': - /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ - $message = __( '%s is invalid. Enter a valid phone number', 'formidable' ); - break; - - case 'number': - /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ - $message = __( '%s is invalid. Enter a number', 'formidable' ); - break; + $type = $field ? FrmField::get_field_type( $field ) : ''; + $messages = array( + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + 'email' => sprintf( __( '%s is invalid. Enter a valid email address, like name@example.com', 'formidable' ), '[field_name]' ), + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + 'url' => sprintf( __( '%s is invalid. Enter a valid web address, like https://example.com', 'formidable' ), '[field_name]' ), + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + 'phone' => sprintf( __( '%s is invalid. Enter a valid phone number', 'formidable' ), '[field_name]' ), + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + 'number' => sprintf( __( '%s is invalid. Enter a number', 'formidable' ), '[field_name]' ), + ); - default: - /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ - $message = __( '%s is invalid', 'formidable' ); + if ( isset( $messages[ $type ] ) ) { + return $messages[ $type ]; } - return sprintf( $message, '[field_name]' ); + /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ + return sprintf( __( '%s is invalid', 'formidable' ), '[field_name]' ); } /** diff --git a/tests/phpunit/fields/test_FrmFieldsHelper.php b/tests/phpunit/fields/test_FrmFieldsHelper.php index 465ee60a9a..bc47406da0 100644 --- a/tests/phpunit/fields/test_FrmFieldsHelper.php +++ b/tests/phpunit/fields/test_FrmFieldsHelper.php @@ -318,45 +318,43 @@ public function test_get_error_msg() { public function test_get_error_msg_invalid_is_field_type_specific() { $form_id = $this->factory->form->create(); - // Email and number fields get their own corrective message when no custom one is set. - $email_field = $this->factory->field->create_and_get( + // Email and number fields get their own corrective message when no custom one is set; + // a type with no specific copy (text) keeps the original generic message. + $tests = array( array( - 'name' => 'Email', - 'form_id' => $form_id, - 'type' => 'email', - ) - ); - - $error_message = FrmFieldsHelper::get_error_msg( $email_field, 'invalid' ); - $this->assertSame( 'Email is invalid. Enter a valid email address, like name@example.com', $error_message ); - - $number_field = $this->factory->field->create_and_get( + 'type' => 'email', + 'name' => 'Email', + 'expected' => 'Email is invalid. Enter a valid email address, like name@example.com', + ), array( - 'name' => 'Age', - 'form_id' => $form_id, - 'type' => 'number', - ) - ); - - $error_message = FrmFieldsHelper::get_error_msg( $number_field, 'invalid' ); - $this->assertSame( 'Age is invalid. Enter a number', $error_message ); - - // Field types with no specific copy keep the original generic message. - $text_field = $this->factory->field->create_and_get( + 'type' => 'number', + 'name' => 'Age', + 'expected' => 'Age is invalid. Enter a number', + ), array( - 'name' => 'Comment', - 'form_id' => $form_id, - 'type' => 'text', - ) + 'type' => 'text', + 'name' => 'Comment', + 'expected' => 'Comment is invalid', + ), ); - $error_message = FrmFieldsHelper::get_error_msg( $text_field, 'invalid' ); - $this->assertSame( 'Comment is invalid', $error_message ); + foreach ( $tests as $test ) { + $field = $this->factory->field->create_and_get( + array( + 'name' => $test['name'], + 'form_id' => $form_id, + 'type' => $test['type'], + ) + ); + + $error_message = FrmFieldsHelper::get_error_msg( $field, 'invalid' ); + $this->assertSame( $test['expected'], $error_message ); + } // A custom message saved on the field is never overridden by the type-specific default. - $text_field->field_options['invalid'] = 'Please fix [field_name]'; + $field->field_options['invalid'] = 'Please fix [field_name]'; - $error_message = FrmFieldsHelper::get_error_msg( $text_field, 'invalid' ); + $error_message = FrmFieldsHelper::get_error_msg( $field, 'invalid' ); $this->assertSame( 'Please fix Comment', $error_message ); } } From b7422119e637dca1814405088a3c0051654468c1 Mon Sep 17 00:00:00 2001 From: "vivi-the-going-merry[bot]" <308115520+vivi-the-going-merry[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 06:04:50 -0600 Subject: [PATCH 3/3] Address Franky's non-blocking review notes: quantity fields (FrmFieldQuantity extends FrmFieldNumber, distinct stored type) were falling back to the generic invalid message; add coverage for url/phone, untested until now. Co-Authored-By: Claude Sonnet 5 --- classes/helpers/FrmFieldsHelper.php | 2 ++ tests/phpunit/fields/test_FrmFieldsHelper.php | 20 +++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/classes/helpers/FrmFieldsHelper.php b/classes/helpers/FrmFieldsHelper.php index 31097ede22..8855a070c5 100644 --- a/classes/helpers/FrmFieldsHelper.php +++ b/classes/helpers/FrmFieldsHelper.php @@ -327,6 +327,8 @@ public static function default_invalid_msg( $field = null ) { /* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */ 'number' => sprintf( __( '%s is invalid. Enter a number', 'formidable' ), '[field_name]' ), ); + // Quantity validates identically to number (FrmFieldQuantity extends FrmFieldNumber) but is a distinct stored type. + $messages['quantity'] = $messages['number']; if ( isset( $messages[ $type ] ) ) { return $messages[ $type ]; diff --git a/tests/phpunit/fields/test_FrmFieldsHelper.php b/tests/phpunit/fields/test_FrmFieldsHelper.php index bc47406da0..cba4b79801 100644 --- a/tests/phpunit/fields/test_FrmFieldsHelper.php +++ b/tests/phpunit/fields/test_FrmFieldsHelper.php @@ -318,19 +318,35 @@ public function test_get_error_msg() { public function test_get_error_msg_invalid_is_field_type_specific() { $form_id = $this->factory->form->create(); - // Email and number fields get their own corrective message when no custom one is set; - // a type with no specific copy (text) keeps the original generic message. + // Email, url, phone, number, and quantity fields get their own corrective message + // when no custom one is set; a type with no specific copy (text) keeps the + // original generic message. $tests = array( array( 'type' => 'email', 'name' => 'Email', 'expected' => 'Email is invalid. Enter a valid email address, like name@example.com', ), + array( + 'type' => 'url', + 'name' => 'Website', + 'expected' => 'Website is invalid. Enter a valid web address, like https://example.com', + ), + array( + 'type' => 'phone', + 'name' => 'Cell', + 'expected' => 'Cell is invalid. Enter a valid phone number', + ), array( 'type' => 'number', 'name' => 'Age', 'expected' => 'Age is invalid. Enter a number', ), + array( + 'type' => 'quantity', + 'name' => 'Amount', + 'expected' => 'Amount is invalid. Enter a number', + ), array( 'type' => 'text', 'name' => 'Comment',