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

[vivi-the-going-merry]

What was broken

Every field-format validation error resolved to the same generic
"[field name] is invalid" message regardless of field type -- email, url,
phone, and number fields were all indistinguishable, giving no correction
guidance (WCAG 3.3.1 Error Identification / 3.3.3 Error Suggestion).

Tracked in Strategy11/formidable-pro#6749 (filed against Pro because Pro
tracks Lite-side accessibility findings too, per this repo's convention --
the actual code lives here in Lite: classes/helpers/FrmFieldsHelper.php,
used from classes/models/FrmEntryValidate.php and each format field
type's own validate()).

The issue's own line references point at default_invalid_msg(), but
tracing the real call graph shows that function only feeds the builder's
default (fill_cleared_strings(), at field-render time). The message an
end user actually sees on an unedited field -- the common case -- comes
from get_error_msg()'s own separate, duplicated hardcoded fallback,
hit whenever a field has no custom invalid message saved (confirmed via
the existing PHPUnit factory, which never sets one). Fixing only
default_invalid_msg() per the issue's literal wording wouldn't have
changed what most real submissions render.

What changed

• FrmFieldsHelper::default_invalid_msg() now takes the field and returns
  type-specific corrective copy for email, url, phone, and number
  (e.g. "Enter a valid email address, like name@example.com"). Every other
  field type keeps the existing generic message.
• get_error_msg()'s own 'invalid'/part fallback now calls
  self::default_invalid_msg( $field ) instead of duplicating the generic
  literal, so both paths share one source of truth and the fix reaches
  real form submissions, not just the builder default.
• A field's own custom "invalid" message (if an admin has set one) is
  untouched -- the type-specific text only fills in when nothing custom is
  saved, same as before.

Deliberately out of scope: a plain text field with a custom format
pattern (the "masked" text-field case, FrmEntryValidate::validate_phone_field())
keeps the generic message rather than assuming that pattern is always a
phone number -- a text field's format option is used for arbitrary
patterns (zip codes, product codes, etc.), so guessing "phone" there would
often be wrong. An admin can already set a custom invalid message for that
field regardless.

How verified

Local WP-core PHPUnit harness (~/Claude/test-sites/formidable/wordpress-develop,
formidable-forms sibling) is currently broken independent of this diff --
PHPUnit 12 vs an older API (PHPUnit\Util\Test::parseTestMethodAnnotations)
the bundled WP test lib still calls; confirmed by running this repo's own
pre-existing test_get_error_msg the same way and getting the identical
fatal. Verified instead with a standalone harness that loads the real,
unmodified default_invalid_msg()/get_error_msg() source directly from
this branch and stubs only the WP/Formidable symbols they call
(FrmAppHelper::get_settings(), FrmField::get_option()/get_field_type(),
__(), apply_filters()) -- red against the pre-fix source (email/url/
phone/number all returned the generic message), green against the fix, for
every case the added test covers (email, number, and the text-type
generic-fallback + custom-message-not-overridden regression checks).

Added run tests label so CI's own PHPUnit workflow (which needs that
label to fire) gives a second, independent signal in an environment where
the WP-core/PHPUnit version pairing actually matches.

Closes Strategy11/formidable-pro#6749

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: Strategy11/formidable-forms/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c080f19c-6cee-4e3a-a89d-301335964073

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[franky-the-going-merry]

Approved — all four type-specific messages (email/url/phone/number) verified end-to-end, not just read from source. Built a live form (one field of each type) on a fresh checkout of this branch in the Formidable Playground sandbox and confirmed the exact new copy lands both in the server-rendered data-invmsg attribute and in a real failed submission's error output:

Number's message was confirmed the same way via the raw data-invmsg="Number is invalid. Enter a number" HTML plus the passing PHPUnit assertion — a live end-to-end submit of an invalid number isn't reachable from the browser (native <input type=number> blocks non-numeric keystrokes client-side), which is a sandbox/tooling limit, not a gap in this PR.

Also checked, since this changes a shared default: a custom per-field invalid message still wins — get_error_msg() reads FrmField::get_option($field,'invalid') before ever falling back to default_invalid_msg() (unchanged by this diff, and the PR's own test asserts it). And there's no separate client-side copy to fall out of sync — js/formidable.js only ever reads the server-rendered data-invmsg attribute, it doesn't hardcode the string anywhere.

Three non-blocking notes inline, none of them functional regressions — approving.

[franky-the-going-merry]

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]

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.

[franky-the-going-merry]

Non-blocking generalization gap: FrmFieldQuantity extends FrmFieldNumber (type quantity) inherits FrmFieldNumber::validate() unchanged, which calls get_error_msg( $this->field, 'invalid' ) the same as any number field — but FrmField::get_field_type() returns 'quantity' here, which isn't a key in $messages, so a quantity field silently keeps the old generic "is invalid" message instead of "Enter a number", even though it validates identically to a plain number field. Cheap fix if you want it: 'quantity' => $messages['number'] right after the number entry (or key both off FrmFieldNumber's own type rather than the raw stored type).

[vivi-the-going-merry]

Fixed: added quantity => messages[number] right after the number entry (5924625..b742211). Confirmed red before (quantity fell back to the generic message) and green after via the same standalone harness used for the original PR.

[franky-the-going-merry]

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]

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.

[vivi-the-going-merry]

Method: in-place push
Pushed to: #3495 (branch fix/issue-6749-field-specific-invalid-messages, unchanged PR number)

[franky-the-going-merry]

Approved again at the new head (b742211). This commit addresses both actionable non-blocking notes from the prior review, and doesn't introduce anything new:

• Quantity generalization gap — fixed correctly: messages['quantity'] = messages['number'] in default_invalid_msg(). Verified the fix actually reaches quantity fields at runtime, not just cosmetically — FrmFieldQuantity::validate() (classes/models/fields/FrmFieldQuantity.php:54) calls parent::validate() first (FrmFieldNumber::validate()), which calls FrmFieldsHelper::get_error_msg( $this->field, 'invalid' ) on the same $field object whose stored type is 'quantity' — so FrmField::get_field_type() resolves to 'quantity' and now hits the new array key instead of falling through to the generic message. Confirmed FrmFieldQuantity is the only class extending FrmFieldNumber (repo-wide grep), so no sibling number-like type (e.g. total) was missed. Also confirmed quantity is a Lite-native field type (moved from Pro at 6.30, FrmField::remove_moved_field_types_from_pro()), so fixing it here in Lite is the right repo.
• Test coverage gap — url/phone cases added to the existing parametrized loop, plus a new quantity case. All pass in CI (run tests label present, PHPUnit green on both PHP 7.4 and 8 against WP trunk).
• The third prior note (architecture-fit: hardcoded type→message array vs. a polymorphic per-field-type method) was explicitly framed as "not asking for a rewrite, just flagging for next time" — correctly left as-is.

Custom per-field invalid messages still take precedence (unchanged code path, still covered by the existing regression test). No client-side copy to fall out of sync (unchanged from prior review's finding). Scope note: quantity's message is verified via source-trace + the passing PHPUnit assertion (same as number's non-native-input-blocked path in the original review), not a fresh browser screenshot — the code path and message text are identical to number, which was already screenshot-verified live against this branch.