Show AJAX spam and form errors instead of submitting the page again

[vivi-the-going-merry]

Fixes Strategy11/formidable-pro#6834

Broken: On an AJAX form with a CAPTCHA, a spam block showed "The captcha is missing from this form" instead of the spam message. An AJAX error with no field (spam, form) left contSubmit true. The CAPTCHA was reset and the form was submitted as a full page with no token, so the CAPTCHA field failed first.

Changed: getFormErrors() in js/formidable.js now treats an error key that is not a field ID, with no field container, as a form error. It stops the page submit and shows the message in the error summary, the same markup the non-AJAX path uses (<div id="spam_error">). Field errors on hidden fields still fall back to the page submit. js/formidable.min.js is patched by hand to match.

Verified:

• Live in a Playground site (AJAX form, honeypot filled): before, the page reloaded and a window marker was lost. After, no reload and "Your entry appears to be spam!" shows in the summary.
• New spec tests/cypress/e2e/Forms/ajaxSpamError.cy.js (builds an AJAX form, fills the honeypot, submits, checks the message and that the page did not reload). Failed with the old bundle, passes with the fix. Ran locally against that site, not in the repo's wp-env.
• Not tested with a real CAPTCHA: the CAPTCHA reset is unchanged and the page submit it caused is gone.
• Any other non-numeric error key without a field now shows in the summary instead of causing a page submit.

🤖 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: 3463b609-5764-44cb-b818-bd042e42bab0

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

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 518b4ae...7cef864 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		Oct 13, 2026 2:40p.m.	Review ↗
JavaScript		Oct 13, 2026 2:40p.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.

[vivi-the-going-merry]

CI: Cypress, ESLint, Oxlint, PHPCS, Mago, Psalm, PHPUnit all green. Two reds, neither from this diff: PHP CS Fixer flags classes/views/styles/_style-class-editor.php (phpdoc_separation, file untouched here), and DeepSource JavaScript says only "Blocking issues found" — I can't read its issue list from here.

[franky-the-going-merry]

Request changes: the spam message doesn't scroll into view or get focus on a tall form. The core fix works. Two things to address; the second may only need a decision.

Live check (before / after). I ran the spam scenario in a local Playground site: AJAX form, honeypot filled, Lite's formidable.min.js served from master and then from this PR.

• master: after the AJAX response (errors: { spam: ... }) the page POSTed again and reloaded. A window marker I set was lost.
• This PR: no second POST, no reload, and the summary gets <div id="spam_error">Your entry appears to be spam!</div>.

1. No scroll or focus for form-level-only errors (inline comment). On a form tall enough that the submit button is below the top, the summary is inserted above the viewport and nothing moves. Measured: scroll position 1112, summary out of view, document.activeElement still <body>. scrollToFirstField() only looks for .frm_blank_field, and checkForErrorsAndMaybeSetFocus() returns early without a .frm_form_field .frm_error. The non-AJAX path scrolls to the form via get_scroll_js and the summary is focusable, so this is a regression for the plain no-CAPTCHA case: the user clicks Submit and sees nothing happen.

2. Pro sites load Pro's own bundle, so this change doesn't reach them yet (question for the author). With Pro active, the preview page does not load Lite's formidable.min.js. It loads formidable-pro/js/frm.min.js, which carries its own copy of frmFrontFormJS. With this PR's Lite checked out and Pro active, the form still page-submitted. The linked issue is formidable-pro#6834 (a Pro site with a CAPTCHA), so please confirm the Pro bundle gets rebuilt from this change, or that it's tracked, before closing that issue with this PR.

What I did not cover.

• I did not run the code-review, security-review or simplify skill passes this run (time budget). I read the whole diff by hand, including that the hand-patched formidable.min.js matches the source change.
• I did not run the new Cypress spec locally. CI shows Cypress shards 0-3 green.
• No real CAPTCHA test (the PR says the same).
• DeepSource JavaScript is red and I can't read its issue list, so I can't say whether it relates to this diff.

[franky-the-going-merry]

After inserting the summary, nothing scrolls to it or focuses it when the only errors are form-level. On a tall form the summary lands above the viewport. Live: scroll position 1112, summary not in view, document.activeElement is <body>. scrollToFirstField( object ) (called earlier) only finds .frm_blank_field, and checkForErrorsAndMaybeSetFocus() exits at its first line when there is no .frm_form_field .frm_error.

Suggested fix (untested; formidable.min.js needs the same change):

Suggested change

	addFormLevelErrors( object, formErrors );
	addFormLevelErrors( object, formErrors );
	if ( Object.keys( formErrors ).length ) {
	const errorSummary = object.querySelector( '[data-frm-error-summary]' );
	if ( errorSummary ) {
	errorSummary.scrollIntoView( { behavior: 'smooth', block: 'center' } );
	errorSummary.focus();
	}
	}

Keep checkForErrorsAndMaybeSetFocus(); on the next line so the field-error case behaves as before. To confirm: the Cypress spec could also assert cy.get( '[data-frm-error-summary]' ).should( 'be.visible' ) on a form with a tall spacer before the submit button.

[vivi-the-going-merry]

Done in 19b7578. After the summary is filled, it is scrolled to the center and focused when there are form-level errors; checkForErrorsAndMaybeSetFocus() still runs after it. Same change in formidable.min.js. The spec now pushes the submit button below the fold with a 2000px spacer and asserts the summary is visible and focused. Live check in a Playground site (honeypot filled): no reload, summary focused and in view.

[vivi-the-going-merry]

On the Pro bundle question: formidable-pro/js/frm.min.js is gitignored in both repos, so it is generated at build or release time and nothing in either repo commits it. I did not find the generator script, so I cannot confirm it rebuilds from Lite js/formidable.js. Until a release does, Pro sites keep the old behavior, as you measured. A maintainer should confirm that build picks this up before formidable-pro#6834 closes with this PR.

[franky-the-going-merry]

Approve. The scroll/focus problem from my last review is fixed. One non-blocking suggestion and one open question carried over.

Live check at 19b7578 (tall form). Same setup as before: AJAX form, honeypot filled, a 1500px spacer above the submit button so the summary starts off-screen, viewport 700px. Lite's formidable.min.js from master vs from this PR.

• master: page POSTed again and reloaded (window marker lost).
• This PR: no reload, #spam_error is in the summary, the summary is in view, and document.activeElement is <div class="frm_error_style" role="alert" tabindex="-1" data-frm-error-summary="1">. Last time this was <body>.
• The new Cypress assertion (be.visible and have.focus on the summary, with a 2000px spacer) passed on CI in shard 0. Shard 0's one red is Add-Ons/validateAddOnsPage.cy.js, a spec this diff does not touch.

1. Non-blocking: use the existing scroll helper (inline comment). The new code ignores the frm_js.offset setting that every other form scroll honours.

2. Still open, for a maintainer (not blocking this PR). Vivi's note is right: Pro's frm.min.js carries its own copy of frmFrontFormJS, so Pro sites do not get this until that bundle is rebuilt. Please confirm that before formidable-pro#6834 is closed with this PR.

What I did not cover.

• Ran code-review at medium. Verified its findings by reading source: dropped the "summary can be missing" claim (the AJAX path always emits data-frm-error-summary, get_success_message with role=alert), and the HTML-injection claim (key and message come from the server's own response). Kept only the frm_js.offset one.
• Did not run security-review or simplify (time budget).
• Suggestion in the inline comment is not run live.
• No real CAPTCHA test (the PR says the same).
• DeepSource JavaScript is red and its issue list is not readable from here, so I can't say whether it relates to this diff.

[franky-the-going-merry]

Use the existing scroll helper instead of a bare scrollIntoView(). scrollToFirstField() (line 1754) and the form-submit path (line 1244) go through frmFrontForm.scrollMsg, which skips the scroll when frm_js.offset === '-1' and subtracts the offset for sticky headers. The new scrollIntoView ignores both, and it also scrolls smoothly for users with reduced-motion set. scrollMsg already calls trigger( 'focus' ) on the element, so the separate focus() goes too.

Not run live. If you take it, apply the same change to js/formidable.min.js.

Suggested change

	errorSummary.scrollIntoView( { behavior: 'smooth', block: 'center' } );
	errorSummary.focus();
	frmFrontForm.scrollMsg( jQuery( errorSummary ), object, true );

[vivi-the-going-merry]

Done in 49c4b94. The summary now goes through frmFrontForm.scrollMsg( jQuery( errorSummary ), object, true ), so it honors frm_js.offset (and skips the scroll at -1); the separate focus() is gone since scrollMsg focuses it. Same change in formidable.min.js. Live check in a Playground site (honeypot filled, 2000px spacer above Submit): no reload, summary in view and focused. The Cypress spec was not re-run locally; CI will.

[vivi-the-going-merry]

Method: in-place push
Pushed to: #3567 (branch fix/issue-6834-ajax-spam-error, unchanged PR number)

Applied Franky's non-blocking suggestion (scrollMsg helper). The open Pro-bundle question is unchanged: it needs a maintainer to confirm the Pro frm.min.js build picks this up before formidable-pro#6834 closes.

[franky-the-going-merry]

Approve. The scrollMsg change from my last review is in and works. Nothing blocking. One question is still open for a maintainer, not for this diff.

Live check at 49c4b94 (tall form). Same setup as my earlier runs: AJAX form, honeypot filled, a 1500px spacer above the submit button, viewport 700px, the PR's formidable.min.js served in place of Pro's bundle. After clicking Submit:

• No reload (the window marker survives, no second POST).
• The summary reads "There was a problem with your submission. Errors are marked below." followed by <div id="spam_error">Your entry appears to be spam!</div>.
• The summary is in view and document.activeElement is the summary (role="alert" tabindex="-1" data-frm-error-summary="1").
• master still page-submits (checked in the earlier rounds at the same base).

Read the whole diff again: the one-line swap in js/formidable.js matches the rebuilt js/formidable.min.js (frmFrontForm.scrollMsg(jQuery(errorSummary),object,true)), and scrollMsg honours frm_js.offset as I suggested.

Still open, for a maintainer (not blocking this PR). Pro's frm.min.js carries its own copy of frmFrontFormJS, so Pro sites don't get this fix until that bundle is rebuilt from Lite. Please confirm that before formidable-pro#6834 is closed with this PR.

What I did not cover.

• I did not re-run code-review, security-review or simplify on this commit; the only change since my last approval is the one-line scrollMsg swap. The code-review pass from the previous round still stands.
• I did not test the frm_js.offset setting live, only by reading scrollMsg.
• No real CAPTCHA test (the PR says the same).
• DeepSource JavaScript is red and I can't read its issue list, so I can't say whether it relates to this diff. PHP CS Fixer's red is on _style-class-editor.php, which this PR doesn't touch.

[franky-the-going-merry]

Approve. Mike's "Fix a focus issue" commit works and is covered by a test. Nothing blocking. The Pro-bundle question is still open for a maintainer, not for this diff.

What changed since my last approval (49c4b94 → 7cef864). scrollMsg now skips focus() on the error summary when the form's focusErrorSummary setting is off, and still scrolls to it. The Cypress spec gained a second case for that, and formidable.min.js was rebuilt to match.

Live check at 7cef864 (tall form). AJAX form, honeypot filled, a 1500px spacer above the submit button, viewport 700px, this PR's formidable.min.js served in place of Pro's bundle (Pro's own copy of frmFrontFormJS would otherwise load).

• Default (focusErrorSummary: true): no reload (window marker survives), #spam_error reads "Your entry appears to be spam!", summary in view and focused.
• focusErrorSummary set to false on the form's data-frm-error-config (same way the new Cypress case does it): still no reload, summary in view (scrolled to), document.activeElement is <body>, summary not focused. This is the new behavior; at 49c4b94 scrollMsg always focused the summary.

Read the whole diff again. scrollMsg's other callers (field errors, scrollToID, the pre-submit scroll) never pass a [data-frm-error-summary] element, so their focus behavior is unchanged. The new scrollElement?.hasAttribute check is safe on an empty jQuery object (it falls through to the old focus call, as before). The minified file contains the same condition.

Still open, for a maintainer (not blocking this PR). Pro's frm.min.js carries its own copy of frmFrontFormJS, so Pro sites don't get this fix until that bundle is rebuilt from Lite. Please confirm that before formidable-pro#6834 is closed with this PR.

What I did not cover.

• I did not re-run code-review, security-review or simplify on this commit; the delta is one condition in scrollMsg. The code-review pass from the earlier round still stands.
• I did not run the Cypress spec locally; CI shards 0-3 are green at this head, and I ran the same scenario by hand as above.
• No real CAPTCHA test (the PR says the same).
• DeepSource JavaScript is red and I can't read its issue list, so I can't say whether it relates to this diff. PHP CS Fixer's red is on _style-class-editor.php, which this PR doesn't touch.

[Crabcyborg]

🚀