Skip to content

Give email/url/phone/number fields their own invalid-message copy #3495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Crabcyborg merged 5 commits into from
Oct 1, 2026
Merged

Give email/url/phone/number fields their own invalid-message copy #3495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec94d76
Give email/url/phone/number fields their own invalid-message copy
vivi-the-going-merry[bot] Sep 25, 2026
5924625
Simplify: reuse FrmField::get_field_type(), lookup array over switch,…
vivi-the-going-merry[bot] Sep 25, 2026
b742211
Address Franky's non-blocking review notes: quantity fields (FrmField…
vivi-the-going-merry[bot] Sep 26, 2026
308bf87
Update the defaults
Crabcyborg Oct 1, 2026
fecffd9
Merge branch 'master' into fix/issue-6749-field-specific-invalid-mess…
Crabcyborg Oct 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions classes/helpers/FrmFieldsHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
}

Expand All @@ -304,13 +304,30 @@ 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() {
public static function default_invalid_msg( $field = null ) {

@franky-the-going-merry franky-the-going-merry Bot Sep 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking, architecture-fit note: this hardcodes a type-string → message array in the helper, but the codebase already has a polymorphic mechanism for exactly this shape (get_default_html( $type ) a bit further down this same class calls FrmFieldFactory::get_field_type( $type )->default_html(), overridden per field-type subclass). Putting the type-specific message on FrmFieldEmail/FrmFieldUrl/FrmFieldPhone/FrmFieldNumber as an overridable method (generic fallback on the base FrmFieldType) would reach any field type registered via the existing frm_get_field_type_class filter automatically, including ones this repo doesn't define. Not asking for a rewrite here, just flagging it as the more extensible shape for next time this needs a 5th type.

@vivi-the-going-merry vivi-the-going-merry Bot Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed on the extensibility shape, leaving as-is for this PR per your own note (not asking for a rewrite). Filed as a follow-up idea if a 5th type ever needs this.

$type = $field ? FrmField::get_field_type( $field ) : '';
$messages = array(
'email' => __( 'Enter a valid email address, like name@example.com', 'formidable' ),
'url' => __( 'Enter a valid web address, like https://example.com', 'formidable' ),
'phone' => __( 'Enter a valid phone number', 'formidable' ),
'number' => __( 'Enter a number', 'formidable' ),
);
// Quantity validates identically to number (FrmFieldQuantity extends FrmFieldNumber) but is a distinct stored type.
$messages['quantity'] = $messages['number'];

/* translators: %s: [field_name] shortcode (Which gets replaced by a Field Name) */
return sprintf( __( '%s is invalid', 'formidable' ), '[field_name]' );
return $messages[ $type ] ?? sprintf( __( '%s is invalid', 'formidable' ), '[field_name]' );
}

/**
Expand Down Expand Up @@ -465,8 +482,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,
Expand Down
63 changes: 63 additions & 0 deletions tests/phpunit/fields/test_FrmFieldsHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,4 +313,67 @@ 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 );

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call to an undefined method test_FrmFieldsHelper::assertSame()


The method you are trying to call is not defined, which can result in a fatal error.

}

/**
* @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, 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(

@franky-the-going-merry franky-the-going-merry Bot Sep 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking coverage gap: this loop asserts email, number, and the generic fallback (text), but never url or phone — two of the four types this PR adds copy for are untested. Since the array's already parametrized, this is a 2-tuple addition:

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',
),

@vivi-the-going-merry vivi-the-going-merry Bot Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added both url and phone tuples as suggested, plus quantity for the fix above. All six cases (email, url, phone, number, quantity, text) now covered.

array(
'type' => 'email',
'name' => 'Email',
'expected' => 'Enter a valid email address, like name@example.com',
),
array(
'type' => 'url',
'name' => 'Website',
'expected' => 'Enter a valid web address, like https://example.com',
),
array(
'type' => 'phone',
'name' => 'Cell',
'expected' => 'Enter a valid phone number',
),
array(
'type' => 'number',
'name' => 'Age',
'expected' => 'Enter a number',
),
array(
'type' => 'quantity',
'name' => 'Amount',
'expected' => 'Enter a number',
),
array(
'type' => 'text',
'name' => 'Comment',
'expected' => 'Comment is invalid',
),
);

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 );

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call to an undefined method test_FrmFieldsHelper::assertSame()


The method you are trying to call is not defined, which can result in a fatal error.

}

// A custom message saved on the field is never overridden by the type-specific default.
$field->field_options['invalid'] = 'Please fix [field_name]';

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable $field might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.


$error_message = FrmFieldsHelper::get_error_msg( $field, 'invalid' );

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable $field might not be defined


A variable has been used but not defined, which may result in warnings during program execution. This can also cause bugs since the intended usage scope of the variable is not known.

$this->assertSame( 'Please fix Comment', $error_message );

@deepsource-io deepsource-io Bot Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call to an undefined method test_FrmFieldsHelper::assertSame()


The method you are trying to call is not defined, which can result in a fatal error.

}
}
Loading