Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

reCaptcha & HTML5 Validation #66

Closed
LiamW opened this issue Apr 29, 2021 · 19 comments
Closed

reCaptcha & HTML5 Validation #66

LiamW opened this issue Apr 29, 2021 · 19 comments

Comments

@LiamW
Copy link

LiamW commented Apr 29, 2021

The invisible mode overtakes a button's click and submits the form first without running the HTML5 validation. So the form tries to submit. Yes you can set SS required on the fields, but the page still reloads with errors. Better UX to have HTML5 validation working.

https://stackoverflow.com/questions/44021400/how-to-run-recaptcha-only-if-html5-validation-has-passed/44026198#44026198

@purplespider
Copy link

I would argue this is more of a bug than an enhancement, as it causes Userforms' javascript validation to be bypassed.

e.g. You have a Userform and have set a field to have a minimum length. If the user has entered fewer characters than required but clicks submit, it briefly says "Please enter at least 10 characters." but submits the form anyway:

Screenshot 2021-07-09 at 13 38 27

@UndefinedOffset
Copy link
Owner

ya I agree @purplespider flagged as such

@UndefinedOffset
Copy link
Owner

@LiamW In my local tests (Firefox and Chrome) pure html5 validation is being triggered before the submit event so nocaptcha does not appear to be involved. I'm curious what browser you were using and what mode of recaptcha (invisible, normal, v3)?

@purplespider In my local testing I do see jQuery validate being triggered that said perhaps it's a load order problem. If userforms.js is loaded after NocaptchaField.js it's possible that it's listeners are not being triggered properly.

Can both of you post what version of userforms you are using (if applicable), nocaptcha, and which version of Silverstripe you are using.

@UndefinedOffset
Copy link
Owner

Also any JS console errors would be helpful

@purplespider
Copy link

Here's an example: https://nocaptcha.pswd.biz/test-form Try submitting the form with less than 10 characters and see what happens.

This is a fresh Silverstripe 4.8.0 install on PHP 7.4, just with the latest versions of userforms and nocaptcha installed:

  • "silverstripe/userforms": "^5.9"
  • "undefinedoffset/silverstripe-nocaptcha": "^2.1"

spam.yml:

SilverStripe\SpamProtection\Extension\FormSpamProtectionExtension:
    default_spam_protector: UndefinedOffset\NoCaptcha\Forms\NocaptchaProtector
UndefinedOffset\NoCaptcha\Forms\NocaptchaField:
    site_key: "XXXX" #Your site key (required)
    secret_key: "XXXX" #Your secret key (required)
    default_theme: "light" #Default theme color (optional, light or dark, defaults to light)
    default_size: "invisible" #Default size (optional, normal, compact or invisible, defaults to normal)
    default_badge: "bottomright" #Default badge position (bottomright, bottomleft or inline, defaults to bottomright)

I then created a user forms page, added a text field and the spam protection field, and set the "Allowed text length" on the text field to "10 to 1000".

Not seeing any errors in the console.


Here is an example page with a single field set as Required, like @LiamW's experience: https://nocaptcha.pswd.biz/test-required-form

You can see if you submit the form without completing the field, the JS validation error appears, but the form still submits, then the server-side validation error message appears.

@UndefinedOffset
Copy link
Owner

@purplespider So i think what's happening in your test instance is one i've seen before in other cases, basically the simple theme in this case is loading another jQuery version which is basically removing the event listeners setup in the one supplied by user forms. I wonder if this is what could be happening in both cases talked about here.

I do see the captcha trying but since the form has already begun to navigate away it's not actually being added to the form. I wouldn't be surprised to see the issue go away if one of the two jQuery versions is removed.

two-jquery-instances

@LiamW
Copy link
Author

LiamW commented Aug 19, 2021

I've been away but I might be able to take a look at this later today. Just a heads up, I was having the same issue with the invisible mode and only 1 jquery version. I wasn't using simple theme, but my custom design.

Would have been latest SS and module version from when I first posted this.

I also don't use the userform module.

@purplespider
Copy link

purplespider commented Aug 20, 2021

I wouldn't be surprised to see the issue go away if one of the two jQuery versions is removed.

@UndefinedOffset Thanks, yep, spot on! I appreciate that this specific issue (in my case at least) isn't actually related to this module now. But would the ideal fix in this instance be to move the jQuery requirement in the "simple" theme from Page.ss to PageController::init() (so it loads before the userforms js files), and stop userforms from adding its own copy of jQuery?

If so, I'll to add a PR to userforms with a config option to disable just the jQuery requirement. (Currently, there is an option to disable all javascript reqs (block_default_userforms_js), but you can't solely disable the jQuery one without using Requirements:block and this is more likely to break with any changes to the jQuery location in future).

@UndefinedOffset
Copy link
Owner

@purplespider Ya that would be what I would think is probably the best bet, prevent userforms from adding it it's own jQuery version at least. I think simple is designed to work out of the box work so it kind of makes sense that it requires it's own version. Technically you can block it in PageController::init() using Requirements::block() and technically you could also do the same with UserForms as well.

@LiamW
Copy link
Author

LiamW commented Aug 20, 2021

@UndefinedOffset Can I send you the site to check in an email? I don't want to post it here. It's on the live site now. I can fill out the form on your website.

@UndefinedOffset
Copy link
Owner

@LiamW perhaps reach out to me via the contact form on https://webbuildersgroup.com/ I honestly hardly check the email on my own site anymore. I'll grab the email when it comes into the inbox for that form, we can connect privately from there.

@UndefinedOffset
Copy link
Owner

UndefinedOffset commented Aug 20, 2021

@LiamW Got your email, in your case it does seem to be that the HTML5 validation is not being allowed to run for some reason.

I do have a theory... I see you have a couple attributes added to the submit button data-sitekey, data-callback, data-action and a class g-recaptcha. You shouldn't need to do that the scripting provided by this library should handle mapping the submit button properly. Which will based on my local tests allow HTML5 validation to work since it's listening on the form submit rather than the clicking of the button. Basically what's happening right now is reCaptcha's own code is kicking in because of those attributes and it is binding to the click of the button rather than the form submit. So removing what I've mentioned here should solve the issue.

I wanted to post here for visibility in case someone runs into a similar problem. But let me know if you want to continue this privately from this point.

@LiamW
Copy link
Author

LiamW commented Aug 20, 2021

Ya I'm fine with discussing here as it makes sense.

As I mentioned in the email, I'm not much of a dev myself. I'm also not doing anything custom to add those attributes in myself. I'm running a super basic form. Attached a screenshot of it so you can see the code minus domain stuff I've removed.

So those attributes are being added by the modules. Either yours or spam protection I guess.

Screen Shot 2021-08-20 at 4 05 16 PM

@UndefinedOffset
Copy link
Owner

UndefinedOffset commented Aug 23, 2021

@LiamW there's definitely something odd going on I'm not seeing this behavior on my local test environment, do you mind posting your composer.json file? As well as your your yaml config for nocaptcha like purplespider posted (make sure to remove the keys). Basically I'm trying to figure out if something else is changing the form, because when viewing the source I'm definitely seeing something adding attributes to the submit button which this module does not do.

Edit: Module does do this when in v3 mode.

@LiamW
Copy link
Author

LiamW commented Aug 23, 2021

Sure no problem.

Screen Shot 2021-08-23 at 12 15 33 PM

And

Screen Shot 2021-08-23 at 12 21 49 PM

@UndefinedOffset
Copy link
Owner

Ah v3 recaptcha seems to be related to that, I'll look into it and let you know what I find, explains why I wasn't seeing it :) I was testing v2 invisible.

@UndefinedOffset
Copy link
Owner

@LiamW can you give master a try and see if that works for you (I assume you have a test version of the affected site). If it does I'll tag a new release.

@LiamW
Copy link
Author

LiamW commented Aug 23, 2021

Yup I believe that has fixed it. Works on my local test site. If I come across anything when I push live or in general, I'll let you know.

Thanks!

@UndefinedOffset
Copy link
Owner

Tagged as 2.1.1

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants