Fix pro issue 1961#2003
Conversation
WalkthroughThe changes in this pull request focus on enhancing the validation process for forms utilizing reCAPTCHA within the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
AbdiTolesa
left a comment
AbdiTolesa
left a comment
There was a problem hiding this comment.
This looks good to me, I found no issues with it.
|
Thanks @AbdiTolesa! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
js/formidable.js (1)
1632-1634: Remove unusedclassListparameter fromsubmitFormNowcallThe
classListparameter is passed tofrmFrontForm.submitFormNow, but it is not used in the function definition. Removing this parameter can prevent confusion and keep the code clean.Apply this diff to remove the unused parameter:
- frmFrontForm.submitFormNow( object, classList ); + frmFrontForm.submitFormNow( object );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- js/formidable.js (3 hunks)
🧰 Additional context used
🔇 Additional comments (1)
js/formidable.js (1)
207-211: Appropriately skip validation for invisible reCAPTCHAThe modification ensures that
validateRecaptchais not called when an invisible reCAPTCHA is present. This aligns with the intended behavior, allowing invisible captchas to process after validation and preventing premature validation failures.
| errors = frmFrontForm.validateFormSubmit( object ); | ||
| if ( Object.keys( errors ).length !== 0 ) { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Provide user feedback when validation fails
If frmFrontForm.validateFormSubmit( object ) returns errors, the function exits without informing the user. To enhance user experience, consider displaying validation errors so users understand why the submission did not proceed.
Would you like assistance in implementing user feedback for validation errors?
| let response; | ||
|
|
||
| const $recaptcha = jQuery( form ).find( '.frm-g-recaptcha' ); | ||
| if ( ! $recaptcha.length ) { | ||
| return errors; | ||
| } | ||
|
|
||
| if ( response.length === 0 ) { | ||
| fieldContainer = $recaptcha.closest( '.frm_form_field' ); | ||
| fieldID = fieldContainer.attr( 'id' ).replace( 'frm_field_', '' ).replace( '_container', '' ); | ||
| errors[ fieldID ] = ''; | ||
| const recaptchaID = $recaptcha.data( 'rid' ); | ||
|
|
||
| try { | ||
| response = grecaptcha.getResponse( recaptchaID ); | ||
| } catch ( e ) { | ||
| if ( jQuery( form ).find( 'input[name="recaptcha_checked"]' ).length ) { | ||
| return errors; | ||
| } | ||
| response = ''; | ||
| } | ||
|
|
||
| if ( response.length === 0 ) { | ||
| const fieldContainer = $recaptcha.closest( '.frm_form_field' ); | ||
| const fieldID = fieldContainer.attr( 'id' ).replace( 'frm_field_', '' ).replace( '_container', '' ); | ||
| errors[ fieldID ] = ''; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling when retrieving reCAPTCHA response
While the try-catch block around grecaptcha.getResponse handles exceptions, consider checking if grecaptcha is defined before attempting to call getResponse. This can prevent unnecessary exceptions and improve the robustness of the code.
Apply this diff to check if grecaptcha is defined:
try {
+ if ( typeof grecaptcha !== 'undefined' ) {
response = grecaptcha.getResponse( recaptchaID );
+ } else {
+ response = '';
+ }
} catch ( e ) {
if ( jQuery( form ).find( 'input[name="recaptcha_checked"]' ).length ) {
return errors;
}
response = '';
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let response; | |
| const $recaptcha = jQuery( form ).find( '.frm-g-recaptcha' ); | |
| if ( ! $recaptcha.length ) { | |
| return errors; | |
| } | |
| if ( response.length === 0 ) { | |
| fieldContainer = $recaptcha.closest( '.frm_form_field' ); | |
| fieldID = fieldContainer.attr( 'id' ).replace( 'frm_field_', '' ).replace( '_container', '' ); | |
| errors[ fieldID ] = ''; | |
| const recaptchaID = $recaptcha.data( 'rid' ); | |
| try { | |
| response = grecaptcha.getResponse( recaptchaID ); | |
| } catch ( e ) { | |
| if ( jQuery( form ).find( 'input[name="recaptcha_checked"]' ).length ) { | |
| return errors; | |
| } | |
| response = ''; | |
| } | |
| if ( response.length === 0 ) { | |
| const fieldContainer = $recaptcha.closest( '.frm_form_field' ); | |
| const fieldID = fieldContainer.attr( 'id' ).replace( 'frm_field_', '' ).replace( '_container', '' ); | |
| errors[ fieldID ] = ''; | |
| } | |
| let response; | |
| const $recaptcha = jQuery( form ).find( '.frm-g-recaptcha' ); | |
| if ( ! $recaptcha.length ) { | |
| return errors; | |
| } | |
| const recaptchaID = $recaptcha.data( 'rid' ); | |
| try { | |
| if ( typeof grecaptcha !== 'undefined' ) { | |
| response = grecaptcha.getResponse( recaptchaID ); | |
| } else { | |
| response = ''; | |
| } | |
| } catch ( e ) { | |
| if ( jQuery( form ).find( 'input[name="recaptcha_checked"]' ).length ) { | |
| return errors; | |
| } | |
| response = ''; | |
| } | |
| if ( response.length === 0 ) { | |
| const fieldContainer = $recaptcha.closest( '.frm_form_field' ); | |
| const fieldID = fieldContainer.attr( 'id' ).replace( 'frm_field_', '' ).replace( '_container', '' ); | |
| errors[ fieldID ] = ''; | |
| } |
2 participants
Fixes https://github.com/Strategy11/formidable-pro/issues/5304
This includes a previous update #1961
Related comment #1961 (comment)
But fixes a bug I was seeing where invisible captchas would fail validation before the validation would happen before the invisible captchas has executed.
I also cleaned up the
validateRecaptchafunction a little. I didn't change any logic. I just have it return early when there are no captchas, and updated the code to useconstmore.I tried testing this with v2 checkbox, invisible, and v3.