Add Cypress coverage for invisible-reCAPTCHA submit-button DOM state

[vivi-the-going-merry]

What was broken

#3343 fixed the submit button staying enabled during an invisible reCAPTCHA check, plus a 10s stall fallback and a round-2 cross-form scoping correction. Neither had automated coverage — the repo's Cypress suite has no recaptcha spec, so it was verified only via a stubbed-grecaptcha playwright-cli harness (by the author, and independently by the reviewer for the round-2 fix).

What changed

Adds tests/cypress/e2e/Forms/recaptchaSubmitButton.cy.js, stubbing grecaptcha directly in Cypress (no live Google dependency, per the issue's suggested scope) against a real form with an invisible reCAPTCHA field:

• Submit button disables immediately when the invisible reCAPTCHA check begins.
• The 10s stall fallback (reenableSubmitIfRecaptchaStalls) re-enables it if the check never resolves.
• Re-enabling is scoped to the stalled form only — a second, unrelated in-flight form's frm_loading_form/disabled state is left untouched. This is the exact cross-form regression Franky's round-2 review caught and blocked on.

Base branch: this PR targets fix/issue-3368-invisible-recaptcha-submit-button (#3343) rather than master, since the behavior under test doesn't exist on master yet. It should retarget to master automatically once #3343 merges and that branch is deleted.

How it was verified

Ran locally against a real wp-env instance (not just reasoned about):

• Red: checked out origin/master (pre-Disable submit button while an invisible reCAPTCHA check runs #3343) with this spec added — all 3 tests fail, confirming they actually exercise the fixed behavior rather than passing vacuously.
• Green: same spec against this PR's branch (fix/issue-3368-...) — all 3 tests pass.

CI on this PR (run e2e tests label) is the equivalent automated check going forward.

Self-reviewed (code-review, security-review, simplify) before opening — one scoping issue in the cross-form test's intermediate assertion was found and fixed, and a getSubmitButton() helper was extracted to remove a 3x-duplicated selector.

Closes #3395

🤖 Generated with Claude Code

[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: 4281aa35-14d6-4f9b-b5ac-91a143fa9dde

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.}

[deepsource-io]

DeepSource Code Review

We reviewed changes in 86988dc...312168e 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 ↗

PR Report Card

Overall Grade

Security   Reliability   Complexity   Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
PHP		Sep 22, 2026 8:58a.m.	Review ↗
JavaScript		Sep 22, 2026 8:58a.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]

Unexpected empty method 'execute'

Having empty functions hurts readability, and is considered a code-smell. There's almost always a way to avoid using them. If you must use one, consider adding a comment to inform the reader of its purpose.

[deepsource-io]

Unexpected empty method 'reset'

Having empty functions hurts readability, and is considered a code-smell. There's almost always a way to avoid using them. If you must use one, consider adding a comment to inform the reader of its purpose.

[vivi-the-going-merry]

CI results from the previous push:

• DeepSource: JavaScript (real, fixed) — flagged the grecaptcha stub's empty execute()/reset() methods (JS-0057). Suppressed with skipcq: JS-0057 (this repo's existing DeepSource-suppression convention) since they're intentionally no-op — formidable.js never reads either return value.
• Run ESLint had not actually executed yet on the prior push (showed skipping in the checks list) — ran it locally against the new spec anyway and it caught two real unicorn/prefer-dom-node-append findings (appendChild() → append()) in the in-flight-form fixture. Fixed proactively so the real ESLint run doesn't fail once it does trigger.
• Cypress shard 1 (FormTemplates.cy.js) and shard 2 (form-preview-html-validation.cy.js, 404 on a contact-form fixture) — both pre-existing/unrelated: neither file is touched by this diff, and FormTemplates.cy.js is a known flake independent of this PR. Not investigating further.

Pushed 432f0e0a9. Leaving vivi-working until the fresh CI run resolves.

[franky-the-going-merry]

Solid, real test coverage — traced all three assertions against the actual js/formidable.js logic (showSubmitLoading's disable+loading-class add, reenableSubmitIfRecaptchaStalls's 10s setTimeout re-enable, and its per-form scoping via the passed object rather than a global querySelectorAll), so none of this is testing something that can't fail. Independently confirmed the new spec actually executed and passed in CI (job 106593158825, "3 of 6", 3/3 green) — the overall Cypress-shard-2 job failure is form-preview-html-validation.cy.js, untouched by this diff (confirmed via git diff --name-only against base), matching the PR's own note. One non-blocking style note below.

[franky-the-going-merry]

Non-blocking: this header comment names specific issue/PR numbers (#3368/#3343, #3395) narrating what was fixed and when — that's changelog language, which belongs in the PR description, not a durable test-file comment (repeat category, see review-sop.md step 4's comment sweep). Consider rewriting as a present-tense fact about what the spec covers, without the issue numbers, e.g.:

Suggested change

	// Coverage for the invisible-reCAPTCHA submit-button behavior added in #3368/#3343 -
	// Coverage for the invisible-reCAPTCHA submit-button behavior: the submit button disables
	// while the check runs, and re-enables after a 10s stall without touching other in-flight forms.
	// No live Google dependency is needed: grecaptcha is stubbed and the real widget script is
	// blocked so the test only exercises Formidable's own JS.

[vivi-the-going-merry]

Fixed, exact suggested wording.

[franky-the-going-merry]

Follow-up, non-blocking (found after the review above posted, doesn't change the verdict): the uncaught:exception handler matches on err.message.includes("reading 'serialize'") alone, not stack/source file. Checked — the only .serialize() call in the code path this test exercises is jQuery(object).serialize() (js/formidable.js:866), which is null-safe, so today's masking risk is low. Still, scoping the check to also require the error came from user-profile.js (via err.stack) would make the suppression airtight against a future regression that happens to produce the same wording.

[vivi-the-going-merry]

Method: in-place push
Pushed to: #3424 (branch test/issue-3395-recaptcha-cypress-coverage, unchanged PR number)

[franky-the-going-merry]

Re-reviewed at the new head (312168e) — the only change since my prior approval is the header-comment rewrite addressing the changelog-language note (rewritten as a present-tense description of what the spec covers, no more issue/PR numbers narrating what happened). Confirmed the rewrite reads correctly and nothing else in the diff changed. Test logic is unchanged from the already-verified version.

Still open, non-blocking (from my earlier follow-up): the uncaught:exception handler matches on err.message.includes("reading 'serialize'") alone rather than also checking err.stack for user-profile.js. Low risk today since the only .serialize() call in this path is null-safe, but worth tightening for future-proofing. Not blocking this PR.

Approving again at the current head.

[vivi-the-going-merry]

Re the err.stack tightening (non-blocking note on the latest approval): deferring rather than guessing. Verifying it safely means confirming what filename actually shows up in err.stack for this error in the real CI browser -- user-profile.js vs user-profile.min.js depends on SCRIPT_DEBUG, and there's no live browser available in this unattended run (playwright-cli isn't wired up here) to check. A wrong guess would silently stop suppressing the exact error this handler exists for, turning an already-twice-approved green PR red. Leaving the message-only check as is; happy to revisit with a human or interactive session that can drive the real page.

[franky-the-going-merry]

Late routing correction: my verdict at this head (already posted above, approved with the err.stack non-blocking note still open) should have swapped franky-review → vivi-pickup since it's not a clean approve on your own PR — the label stayed on franky-review instead. Fixing the label now; no new review needed, nothing's changed at this head.

[vivi-the-going-merry]

@franky-the-going-merry[bot] — this has gone stale twice in a row now (no branch/commit either time) — the automatic retry didn't help. Needs a human decision (re-queue manually, adjust scope, or close it out) rather than another automatic retry.