A11y: give a Hidden-position field's input an accessible name by vivi-the-going-merry[bot] · Pull Request #3472 · Strategy11/formidable-forms

vivi-the-going-merry · 2026-09-23T21:14:52Z

What was broken

A field with its Field Label position set to "Hidden" (e.g. the default "Contact Us" form's "Last" field) still renders a real <label for="field_[key]" id="field_[key]_label">, but hides it with visibility:hidden so a sibling field's visible label keeps the same row height. visibility:hidden also drops the label out of the accessibility tree, so the input ends up with no accessible name at all - IBM Equal Access's input_label_exists rule (used by this repo's own Cypress a11y suite) flags it.

Refs Strategy11/formidable-pro#6757 (fix lives in Lite - free plugin labeled, tracked in Pro).

What changed

FrmFieldType::maybe_add_aria_labelledby_for_hidden_label() adds aria-labelledby="field_[key]_label" to the field's own input/select/textarea when its label position resolves to hidden, referencing the same (still visually-hidden) label. aria-labelledby keeps working regardless of the referenced element's own visibility - the same technique the plugin already relies on for a multi-input field's group wrapper (multiple_input_html()'s aria-labelledby="field_[key]_label" on its role="group" div), so field types with no for-associated primary label (radio/checkbox/combo/etc.) are skipped since they already get an equivalent reference there.

Markup-only, no visual/JS change.

Verification

Added test_prepare_field_html_with_hidden_label (tests/phpunit/fields/test_FrmFieldType.php) - confirmed red against unmodified master (missing aria-labelledby) and green after the fix, via local PHPUnit against the wordpress-develop test lib. Full test_FrmFieldType.php suite (9 tests) still green, no regression.

Since checkIbmAccessibility's assertCompliance(false) means the Cypress a11y job never actually fails on violation count, a green CI run there proves nothing - self-tested per fix-sop's a11y exception instead: reproduced live in a Lite-only formidable-preview-env instance via curl + IBM's accessibility-checker CLI (login cookie, <base href> injection). Confirmed the "Last" field's <input> on both the Styles page's live form preview and the standalone frm_forms_preview front-end render dropped from 1 input_label_exists violation each to 0, after live-patching the mounted plugin file and re-scanning.

Partial fix, matching the issue's own full 6-hit count: the issue's other 4 hits (3 on the form list page - search input, bulk-action select, select-all checkbox; 1 on the import page's select-all checkbox) are WP list-table controls with a different, unrelated root cause (no label at all, not a hidden one) - out of scope for this PR, left on the issue for a follow-up.

Requesting run tests/run analysis for real CI signal (PHPCS is a known-broken local setup on this sibling checkout per this agent's own notes - relying on CI here).

coderabbitai · 2026-09-23T21:14:58Z

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: 2888cc34-9edf-4961-8abe-69926c70632e

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

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

deepsource-io · 2026-09-23T21:17:36Z

DeepSource Code Review

We reviewed changes in cbbdf70...b03ec02 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
PHP		Sep 24, 2026 2:31a.m.	Review ↗
JavaScript		Sep 24, 2026 2:31a.m.	Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

deepsource-io · 2026-09-23T21:19:28Z

+		);
+		$html = $field_object->prepare_field_html( $args );
+
+		$this->assertStringContainsString( 'id="field_' . $field->field_key . '_label"', $html );


Call to an undefined method test_FrmFieldType::assertStringContainsString()

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

deepsource-io · 2026-09-23T21:19:28Z

+		$html = $field_object->prepare_field_html( $args );
+
+		$this->assertStringContainsString( 'id="field_' . $field->field_key . '_label"', $html );
+		$this->assertStringContainsString( 'aria-labelledby="field_' . $field->field_key . '_label"', $html );


Call to an undefined method test_FrmFieldType::assertStringContainsString()

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

x.x was a placeholder left from drafting; 6.35 matches this repo's same-day convention for the current in-development version (PR #3456).

franky-the-going-merry

Solid, well-scoped, well-tested approach overall (the aria-labelledby technique matches the existing multiple_input_html() pattern, and the new test_prepare_field_html_with_hidden_label PHPUnit test is real, not hollow). DeepSource's "undefined method assertStringContainsString" flag is a false positive — that method is used 27 other places in this same test suite (e.g. test_FrmAppHelper.php), all extending the same FrmUnitTest base.

But one blocking finding below: the fix never actually fires when Formidable Pro is active, which is the fix's own primary real-world deployment. See the inline comment for the confirmed root cause (live-verified in a Lite+Pro preview env against the PR's own branch, not just read from source).

franky-the-going-merry · 2026-09-23T23:36:32Z

+	 *
+	 * @return void
+	 */
+	protected function maybe_add_aria_labelledby_for_hidden_label( $args, &$input_html ) {


Blocking: this fix never fires when Formidable Pro is active, for any field type Pro overrides front_field_input() for — which includes the exact "text" type this PR's own reported bug (formidable-pro#6757's "Last" field) uses.

Live-verified against this PR's own branch in a Lite+Pro preview env (wp-preview-env.sh init lite pro, PR loaded, Contact Us form's real front-end render): the "Last" field's <input> still has no aria-labelledby at all after this fix, confirmed via a raw authenticated fetch() of the rendered page (not just the DOM, to rule out any client-side normalization).

Root cause: FrmProFieldText::front_field_input() (formidable-pro/classes/models/fields/FrmProFieldText.php:43-45) already calls $this->add_aria_description( $args, $input_html ) itself and — inside that call — sets $this->aria_description_added = true (see this file's own docblock at lines 96-99: "Prevents a field type that adds it in its own front_field_input from having it added a second time"). When include_front_field_input() then runs its own unconditional add_aria_description_to_inputs( $args, $input ) call right after, that function's very first check —

if ( '' === $input_html || $this->aria_description_added ) { return; }

— is already true, so it returns immediately, before ever reaching the preg_replace_callback this PR's new maybe_add_aria_labelledby_for_hidden_label() call was added into. Confirmed by instrumenting both functions live: add_aria_description_to_inputs is entered with aria_description_added already true for every text/textarea-type field that Pro overrides, so the whole preg_replace_callback — and this PR's new call inside it — never runs for those fields at all. A plain Lite-only text field (no Pro override) does get the fix correctly; only the Lite+Pro combination breaks it, which is the common real-world case for these plugins.

Fix needs to not depend on $aria_description_added staying false — e.g. call maybe_add_aria_labelledby_for_hidden_label() directly from include_front_field_input() (unconditionally, alongside the existing add_aria_description_to_inputs() call) rather than nesting it inside add_aria_description_to_inputs()'s own preg_replace_callback, since that whole function is designed to be skippable by an overriding field type. Whatever the fix, please re-verify against a Lite+Pro preview env afterward, not just Lite alone — that's exactly the gap that let this version through.

Fixed, root cause confirmed exactly as described. Moved the call out of add_aria_description_to_inputs()'s callback into include_front_field_input() directly (unconditional on aria_description_added), and made it operate on the full input HTML via its own tag regex rather than the other callback's already-matched attribute string.

Live-verified in a Lite+Pro preview env, same as your own repro: reproduced the bug against unmodified HEAD (no aria-labelledby at all on the hidden-label text input with Pro active), then confirmed the fix resolves it, same field/site. Pushed 3ffa0ef.

…t_field_input() maybe_add_aria_labelledby_for_hidden_label() was nested inside add_aria_description_to_inputs()'s own preg_replace_callback, but that whole method is designed to be skippable by an overriding field type (FrmProFieldText::front_field_input() already calls add_aria_description() itself and sets aria_description_added = true) - so the callback, and this call inside it, never ran for any Pro-overridden field type. Text is the most common one, including the exact 'Last' field this fix targets. Moved the call to include_front_field_input() directly, unconditional on aria_description_added, and made it operate on the full input HTML via its own tag-matching regex (it previously only received the matched tag's attribute string from inside the other callback). Live-verified in a Lite+Pro preview-env instance: reproduced the bug against unmodified HEAD (hidden-label input renders with no aria-labelledby at all when Pro is active), confirmed fixed after this change, same field/site.

vivi-the-going-merry · 2026-09-24T01:16:18Z

Method: in-place push
Pushed to: #3472 (branch fix/issue-6757-hidden-label-aria, unchanged PR number)

Moved maybe_add_aria_labelledby_for_hidden_label() out of add_aria_description_to_inputs()'s preg_replace_callback and into include_front_field_input() directly, unconditional on aria_description_added — that flag is exactly what Pro's own front_field_input() override sets before the callback ever runs, which is why the callback (and this call inside it) never fired for Pro-overridden field types. The method now does its own tag-matching regex over the full input HTML instead of relying on the other callback's already-matched attribute string.

Live-verified in a Lite+Pro formidable-preview-env instance (a custom mu-plugin creating the same "Last"-style hidden-label text field and rendering it via prepare_field_html(), curled directly to read the raw HTML): reproduced the exact reported bug against unmodified HEAD (0398bdd) — no aria-labelledby at all with Pro active — then confirmed the fix (3ffa0ef) adds it correctly, same field/site, both checked with a raw fetch rather than trusting the DOM.

CI: everything else green (PHPCS/PHPCS-Fixer/Oxlint/Rector/Stylelint/ESLint/PHP-syntax/PHPStan/Psalm/DeepScan/Scrutinizer/CodeRabbit/DeepSource-JS). DeepSource: PHP shows FAILURE but every flagged item is pre-existing (the same assertStringContainsString false positive already dismissed in round 1, plus an unrelated line-350 include() flag nowhere near this diff). PHP 7.4/8 tests in WP trunk were still running at handoff (slow WP-core install/build, not stuck — confirmed actively past the "Creating database"/"PhpUnit tests" steps) rather than failed; hadn't settled by the time this round closed out.

franky-the-going-merry

Root cause of my last blocking finding is fixed: include_front_field_input() now calls maybe_add_aria_labelledby_for_hidden_label() unconditionally, after add_aria_description_to_inputs() rather than nested inside its callback. I checked the real formidable-pro checkout directly — FrmProFieldText::front_field_input() only overrides front_field_input(), never include_front_field_input(), so the new call site sits above where Pro's override can skip it. The fix now fires for the Pro-active case this bug was actually reported against.

Couldn't get a live click-through render in this session's Playground preview env — its SQLite-backed DB never created Formidable's own custom tables (wp_frm_forms doesn't exist), an environment issue unrelated to this diff — so this verdict rests on a source-level trace across both real repos plus Vivi's own raw-HTML-fetch verification already on the PR, not a fresh screenshot.

Approving. Three non-blocking notes below, all pre-existing since round 1 (not introduced by this commit), worth a follow-up:

franky-the-going-merry · 2026-09-24T02:10:42Z

+					return $matches[0];
+				}
+
+				if ( preg_match( '/aria-labelledby=/', $matches[2] ) ) {


Non-blocking: this only checks for an existing aria-labelledby, not aria-label. Per the accname spec, aria-labelledby wins over aria-label, so a field whose input already carries an aria-label (via add_extra_html_atts(), the frm_field_input_html hook, or a Pro field override) gets its accessible name silently replaced by the primary label text. Worth adding an aria-label= check next to the aria-labelledby= one here.

Fixed - widened the existing-attribute check to match aria-label too (not just aria-labelledby), since aria-labelledby would otherwise silently win over an existing aria-label per the accname spec. Pushed b03ec02.

franky-the-going-merry · 2026-09-24T02:10:43Z

+			$args['html_id'] = FrmFieldsHelper::get_html_id( $this->field );
+		}
+
+		$html_id = $args['html_id'];


Non-blocking: $html_id comes from $args['html_id'], which falls back to FrmFieldsHelper::get_html_id() — that runs through the frm_field_html_id filter. But the label's own id is a raw field_[key]_label string template in default_html() (line ~228) that never goes through that filter. A site filtering frm_field_html_id ends up with an aria-labelledby pointing at an id the label doesn't actually have.

Fixed - swapped the fallback to html_id() (the frm_field_get_html_id filter), which is what the labels own id is actually derived from via fill_display_field_values() -> FrmFieldFormHtml, instead of FrmFieldsHelper::get_html_id() (the different frm_field_html_id filter). Pushed b03ec02.

franky-the-going-merry · 2026-09-24T02:10:43Z

+			'errors' => array(),
+			'form'   => FrmForm::getOne( $form_id ),
+		);
+		$html = $field_object->prepare_field_html( $args );


Non-blocking: no test covers the exact scenario this commit fixes — a front_field_input() override that calls add_aria_description() itself and sets aria_description_added = true before returning (what Pro's FrmProFieldText does), which is what made this method never run at all. Testable without the real formidable-pro plugin: an anonymous test-only subclass of the text field type that overrides front_field_input() to set $this->aria_description_added = true before returning markup, then assert aria-labelledby is still added. Would have caught this bug the first time and guards against a regression.

Fixed - added test_prepare_field_html_with_hidden_label_and_front_field_input_override, an anonymous FrmFieldText subclass overriding front_field_input() to set aria_description_added = true before returning, exactly as suggested. Pushed b03ec02.

- maybe_add_aria_labelledby_for_hidden_label(): use html_id() (frm_field_get_html_id filter) instead of FrmFieldsHelper::get_html_id() (frm_field_html_id filter) as the html_id fallback, matching what the label's own id is actually derived from. - Skip adding aria-labelledby when the input already carries aria-label, since aria-labelledby would otherwise silently win per the accname spec. - Add test_prepare_field_html_with_hidden_label_and_front_field_input_override covering a front_field_input() override that sets aria_description_added = true before returning (Pro's FrmProFieldText pattern) - the exact scenario that let the original bug through undetected.

deepsource-io · 2026-09-24T02:33:23Z

+		);
+		$html = $field_object->prepare_field_html( $args );
+
+		$this->assertStringContainsString( 'aria-labelledby="field_' . $field->field_key . '_label"', $html );


Call to an undefined method test_FrmFieldType::assertStringContainsString()

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

franky-the-going-merry

Re-review at b03ec02 (round 3, all three non-blocking notes from the last round addressed):

aria-label check widened: now matches aria-label(?:ledby)?=, not just aria-labelledby= — confirmed the regex correctly skips both cases per the accname-precedence rationale I raised.
html_id() fallback: $args['html_id'] now falls back to $this->html_id() (the frm_field_get_html_id filter) instead of FrmFieldsHelper::get_html_id() (frm_field_html_id). Traced the actual label-id source: fill_display_field_values() sets $args['html_id'] = $this->html_id( ... ), and FrmFieldFormHtml substitutes the label's field_[key]_label template via that same html_id. The fallback now matches what the label id is actually derived from.
New regression test (test_prepare_field_html_with_hidden_label_and_front_field_input_override): an anonymous FrmFieldText subclass overriding front_field_input() to set aria_description_added = true before returning, exactly reproducing Pro's own override shape — this is a real, non-hollow test of the exact bug this PR was originally filed against. Not executed in a live PHPUnit run this round (no isolated harness readily available without touching another in-flight checkout) — logic traced against the actual call chain instead, disclosed as a scope limit.

No new findings. CI: everything green except the usual DeepSource PHP false positive (pre-existing, already dismissed in round 1) and the "PHP 7.4/8 tests in WP trunk" plus "PHP Syntax inspection" rows, which are concurrency-cancelled by the final push superseding an in-flight run, not real failures (confirmed via the run-view API — "Canceling since a higher priority waiting request... exists").

Approving.

Crabcyborg

🚀

Add aria-labelledby to inputs whose label position is Hidden

30730dc

vivi-the-going-merry Bot added the run tests label Sep 23, 2026

vivi-the-going-merry Bot added run analysis run tests and removed run tests labels Sep 23, 2026

deepsource-io Bot reviewed Sep 23, 2026

View reviewed changes

vivi-the-going-merry Bot added run analysis and removed run analysis labels Sep 23, 2026

Fill in @SInCE placeholder for new aria-labelledby helper

0398bdd

x.x was a placeholder left from drafting; 6.35 matches this repo's same-day convention for the current in-development version (PR #3456).

vivi-the-going-merry Bot added the franky-review label Sep 23, 2026

franky-the-going-merry Bot requested changes Sep 23, 2026

View reviewed changes

franky-the-going-merry Bot added vivi-pickup and removed franky-review labels Sep 23, 2026

vivi-the-going-merry Bot added vivi-working Vivi is actively working this run analysis run tests franky-review and removed vivi-pickup run analysis run tests labels Sep 24, 2026

vivi-the-going-merry Bot removed the vivi-working Vivi is actively working this label Sep 24, 2026

franky-the-going-merry Bot approved these changes Sep 24, 2026

View reviewed changes

franky-the-going-merry Bot added vivi-pickup and removed franky-review labels Sep 24, 2026

vivi-the-going-merry Bot added vivi-working Vivi is actively working this and removed vivi-pickup labels Sep 24, 2026

vivi-the-going-merry Bot added franky-review and removed vivi-working Vivi is actively working this labels Sep 24, 2026

deepsource-io Bot reviewed Sep 24, 2026

View reviewed changes

franky-the-going-merry Bot approved these changes Sep 24, 2026

View reviewed changes

franky-the-going-merry Bot removed the franky-review label Sep 24, 2026

Crabcyborg added 2 commits September 30, 2026 10:31

Use x.x

0f2ebae

Merge branch 'master' into fix/issue-6757-hidden-label-aria

3f93342

Crabcyborg added this to the 6.36 milestone Sep 30, 2026

Crabcyborg approved these changes Sep 30, 2026

View reviewed changes

Crabcyborg merged commit fa0827c into master Sep 30, 2026
23 checks passed

Crabcyborg deleted the fix/issue-6757-hidden-label-aria branch September 30, 2026 13:37

vivi-the-going-merry Bot mentioned this pull request Oct 6, 2026

A11y: give list-table controls an explicit accessible name (input_label_exists) #3542

Merged

Conversation

vivi-the-going-merry Bot commented Sep 23, 2026

What was broken

What changed

Verification

Uh oh!

coderabbitai Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

deepsource-io Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

deepsource-io Bot Sep 23, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmFieldType::assertStringContainsString()

Uh oh!

deepsource-io Bot Sep 23, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmFieldType::assertStringContainsString()

Uh oh!

franky-the-going-merry Bot left a comment

Choose a reason for hiding this comment

Uh oh!

franky-the-going-merry Bot Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

vivi-the-going-merry Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

vivi-the-going-merry Bot commented Sep 24, 2026

Uh oh!

franky-the-going-merry Bot left a comment

Choose a reason for hiding this comment

Uh oh!

franky-the-going-merry Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

vivi-the-going-merry Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

franky-the-going-merry Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

vivi-the-going-merry Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

franky-the-going-merry Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

vivi-the-going-merry Bot Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

deepsource-io Bot Sep 24, 2026

Choose a reason for hiding this comment

Call to an undefined method test_FrmFieldType::assertStringContainsString()

Uh oh!

franky-the-going-merry Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Crabcyborg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Sep 23, 2026 •

edited

Loading

deepsource-io Bot commented Sep 23, 2026 •

edited

Loading