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.

Sign up for GitHub

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

Jump to bottom

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
Open

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

wants to merge 6 commits into from

Conversation

Bashev
Copy link

@Bashev Bashev commented Dec 19, 2023 •
edited
Loading

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)

This was referenced Dec 19, 2023
Open
Open
@Bashev
Copy link
Author

Bashev commented Dec 19, 2023

@magento give me 2.4-develop instance

@magento-deployment-service Magento Deployment Service
Copy link

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

@magento-deployment-service Magento Deployment Service
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

@magento-deployment-service Magento Deployment Service
Copy link

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

@magento-deployment-service Magento Deployment Service
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 20, 2023

@magento run all tests

@magento-automated-testing Magento Automated Testing
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

@magento-automated-testing Magento Automated Testing
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 •
edited
Loading

@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

@Bashev
Copy link
Author

Bashev commented Dec 20, 2023

@magento run Functional Tests CE, Functional Tests EE

@magento-automated-testing Magento Automated Testing
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

@magento-automated-testing Magento Automated Testing
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 •
edited
Loading

@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 •
edited
Loading

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ihor-sviziev ihor-sviziev ihor-sviziev approved these changes

@fredden fredden fredden approved these changes

Assignees

@ihor-sviziev ihor-sviziev

Labels
Progress: approved
Projects
None yet
Milestone
No milestone
3 participants
@Bashev @ihor-sviziev @fredden