Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Load recaptcha related files on focus of the form fields. #334

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Bashev
Copy link

@Bashev Bashev commented Dec 19, 2023

Description (*)

reCaptcha remote files will be loaded only if customer (visitor) focus on the fields for the form for which reCaptcha is enabled. This will reduce loaded files and immediately reflects to loading time.

Fixed Issues (if relevant)

  1. Fixes reCapcha loading reflects to page speed #333
  2. Fixes [Performance] Load reCaptcha on focus to input to improve page load times magento2#38303

Manual testing scenarios (*)

  1. Enable reCaptcha (no matter which version) for Newsletter, Contact Form, Review, Registration or ... anything.
  2. Load the page which will contains form for which reCaptcha is enabled (Ex. Contact form /contact)
  3. ReCaptcha external (gstatic.com) files will not be loaded (you will not see the logo of the recaptcha or loaded files in the network tab of the browser developer inspector).
  4. Click on the first or random field (Ex. Name)
  5. Recaptcha files will be loaded and you will see logo of the repCaptcha somewhere on the page as you are configured in the step 1.

Questions or comments

Contribution checklist (*)

  • Author has signed the Adobe CLA
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@Bashev
Copy link
Author

Bashev commented Dec 19, 2023

@magento give me 2.4-develop instance

Copy link

Hi @Bashev. Thank you for your request. I'm working on Magento instance for you.

Copy link

Hi @Bashev, unfortunately there is no ability to deploy Magento instance at the moment. Please try again later.

@Bashev
Copy link
Author

Bashev commented Dec 19, 2023

@magento give me 2.4-develop instance

Copy link

Hi @Bashev. Thank you for your request. I'm working on Magento instance for you.

Copy link

Hi @Bashev, unfortunately there is no ability to deploy Magento instance at the moment. Please try again later.

@m2-community-project m2-community-project bot moved this from Ready for Review to Reviewer Approved in Pull Request Progress Dec 19, 2023
@Bashev
Copy link
Author

Bashev commented Dec 20, 2023

@magento run all tests

Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@Bashev
Copy link
Author

Bashev commented Dec 20, 2023

@magento run all tests

Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Dec 20, 2023

@Bashev, unfortunately, I don't have write permissions to this repo.
Could you please change

Fixed Issues (if relevant)
1. https://github.com/magento/security-package/issues/333
2. https://github.com/magento/magento2/issues/38303

to

Fixed Issues (if relevant)
1. Fixes https://github.com/magento/security-package/issues/333
2. Fixes https://github.com/magento/magento2/issues/38303

so that two issues will be automatically closed when this PR will be merged

@m2-community-project m2-community-project bot moved this from Reviewer Approved to Ready for Review in Pull Request Progress Dec 20, 2023
@Bashev
Copy link
Author

Bashev commented Dec 20, 2023

@magento run Functional Tests CE, Functional Tests EE

Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@Bashev
Copy link
Author

Bashev commented Dec 20, 2023

@ihor-sviziev @fredden, I think the error from the failed test is not related to the PR.

@Bashev
Copy link
Author

Bashev commented Jan 3, 2024

@magento run Functional Tests CE

Copy link

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

@ihor-sviziev
Copy link
Contributor

ihor-sviziev commented Jan 3, 2024

@Bashev, I saw the comment from @techtoni in magento/magento2#38303 (comment):

Re invisible reCaptcha v3 - the way this technology works is it tracks the user behaviour through the website to establish if it is real user or a bot. It is supposed to be included on all pages, not loaded after a form is interacted with, see documentation - https://developers.google.com/recaptcha/docs/v3

It looks like this PR actually changes behavior for v2 invisible and v3 invisible, which means v3 might work incorrectly due to this change. Could you please double-check that?

@Bashev
Copy link
Author

Bashev commented Jan 3, 2024

@ihor-sviziev i saw the comment also, and also read the documentation.

reCAPTCHA works best when it has the most context about interactions with your site, which comes from seeing both legitimate and abusive behavior. For this reason, we recommend including reCAPTCHA verification on forms or actions as well as in the background of pages for analytics.

Works best not means, it's mandatory. From my point of view this not break the rules of reCAPTCHA. Yes probably this will have some negative impact of the scoring, but will be acceptable.

All of us knows, Google uses reCAPTCHA also to track user behavior and this is the main reason for which they want to have it on all pages.

@Bashev
Copy link
Author

Bashev commented Apr 6, 2024

@magento run Functional Tests CE

Co-authored-by: Ihor Sviziev <ihor-sviziev@users.noreply.github.com>
@m2-community-project m2-community-project bot moved this from Review in Progress to Reviewer Approved in Pull Request Progress May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Pull Request Progress
  
Reviewer Approved
3 participants