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

Checkbox IDs for Terms and Conditions should be unique in Checkout #6207

Closed
bka opened this issue Aug 17, 2016 · 15 comments
Closed

Checkbox IDs for Terms and Conditions should be unique in Checkout #6207

bka opened this issue Aug 17, 2016 · 15 comments
Assignees
Labels
bug report Component: Checkout Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Release Line: 2.1

Comments

@bka
Copy link
Contributor

bka commented Aug 17, 2016

Magento_CheckoutAgreements places checkboxes into checkout to accept the respective terms and conditions. However, the html tag for the checkbox appears for each payment method with the same html id. It is bad practice to have the same html id multiple times and also introduces problems with css customization for these boxes which relies on unique ids.

Preconditions

  1. Install Magento 2.1

Steps to reproduce

  1. Create a Term and Condition under Stores -> Terms and Conditions
  2. Activate Terms and Conditions under Stores -> SALES -> Checkout -> Enable Terms and Conditions
  3. Enable a second payment method e.g. Bank Transfer Payment
  4. Got through checkout process to Review & Payment
  5. Inspect HTML of checkbox tag for configured term

Expected result

  1. Each input field of type checkbox should have a unique html id

Actual result

  1. HTML contains 2 input fields of type checkbox with same id agreement_1

auswahl_006

bka added a commit to bka/magento2 that referenced this issue Aug 17, 2016
bka pushed a commit to bka/magento2 that referenced this issue Aug 18, 2016
@bka
Copy link
Contributor Author

bka commented Aug 18, 2016

Here is an example of customized checkboxes: http://fiddle.jshell.net/78n9pr65/
This demonstrates, that the second is not useable, if input id appears more than once.
With unique ids it works perfectly well.

@SerhiyShkolyarenko SerhiyShkolyarenko added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Nov 16, 2016
@SerhiyShkolyarenko
Copy link
Contributor

Thank you for reporting the issue. Our internal on is MAGETWO-61048.

@mavr1982
Copy link

Solved in 2.1.4 EE?

@mavr1982
Copy link

mavr1982 commented Mar 7, 2017

What kind of support is this?

@mpont91
Copy link

mpont91 commented Mar 16, 2017

2.1.5 and this bug still exists...

@bka
Copy link
Contributor Author

bka commented May 20, 2017

Just checked, this Issue is still present in latest develop branch.
auswahl_102

There are 4 matches for agreement_1, and html id="agreement_1" is appearing twice in my test case.

bka added a commit to teamneusta/magento2 that referenced this issue May 21, 2017
bka added a commit to teamneusta/magento2 that referenced this issue May 21, 2017
magento-team pushed a commit that referenced this issue May 27, 2017
… more unique #6207 #9717

 - Merge Pull Request #9717 from teamneusta/magento2:issue-6207
magento-team pushed a commit that referenced this issue May 27, 2017
[EngCom] Public Pull Requests
 - MAGETWO-69379 use payment method name to make checkbox of agreements more unique #6207 #9717
 - MAGETWO-69378 #4272: v2.0.4 Credit memos with adjustment fees cannot be fully refunded with a second credit memo #9715
 - MAGETWO-69375 Can't delete last item in cart if Minimum Order is Enable #6151 #9714
 - MAGETWO-69230 #7279 bill-to name and ship-to name truncated to 20 chars #9654
 - MAGETWO-69155 Fix coding standard in Magento AdminNotification module #9627
@ishakhsuvarov
Copy link
Contributor

Closing, as issue is fixed in develop branch with the linked commits.

@korostii
Copy link
Contributor

@ishakhsuvarov, I hate to underline the obvious, but if the bug was discovered on 2.1.x and then reportedly "fixed", it is commonly expected to see that fix in the closest patch (2.1.x) release.

To be honest, I don't see how is "merged to develop branch" an acceptable resolution to an acknowledged bug. Until the fix is officially released, a lot of the users are still struggling with it.
In my opinion, having these patches added into the nearest 2.1.x (or 2.1.x.x) releases right away would be perfect, but that doesn't seem to be happening, so...

Could you please at least include a short piece of information that will explain to an ordinary merchant, how he\she can benefit from such s fix?
Something along the following lines (not sure which of these is recommended by Magento, Inc.):

  • patch the core files according to the commits referenced (make sure to backup the patch files and re-apply them every time you update the Magento to the next released version)
  • fork the repository, and cherry-pick these commits into your local 2.1 branch and use that instead of a downloaded or composer-installed version (Although I believe this was not officially recommended if you don't plan to contribute to the project)
  • the option above plus make a pull request pointing back into 2.1-develop
  • write a module that'll do these changes "the right way" using all those shiny extends and overrides and plugins and observers and so on in order to avoid patching the core directly

Keeping the issue open until the fix appears in a 2.1.x release is also a feasible option.

@ishakhsuvarov
Copy link
Contributor

ishakhsuvarov commented May 29, 2017

Keeping the issue open until the fix appears in a 2.1.x release is also a feasible option.

@korostii Reopening, thank you for pointing out my mistake.

@ishakhsuvarov ishakhsuvarov reopened this May 29, 2017
@korostii
Copy link
Contributor

korostii commented May 29, 2017

No problem, keep up the good work!

@magento-team magento-team added Release Line: 2.1 Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Checkout develop labels Jul 31, 2017
@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-69379

@magento-engcom-team magento-engcom-team added Release Line: 2.1 Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Checkout develop Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Fixed in 2.2.x The issue has been fixed in 2.2 release line labels Sep 11, 2017
@gerben86
Copy link

I still encounter this issue in 2.1.10. How should I solve this?

hostep pushed a commit to hostep/magento2 that referenced this issue Feb 7, 2018
… make checkbox of agreements more unique magento#6207 magento#9717

(cherry picked from commit 14b9b98)
@Rekha-periasamy
Copy link

I have tried to apply #13543 and #6207 patch file for fixing the terms and conditions issue in web shop. Still I'm facing issue.

Steps to reproduce:

  1. Enable check money order and Paypal

  2. In checkout page check the check/money order payment method checkbox and go to Paypal payment method then hit the place order button. The expected behavior should be throw the validation message in Paypal payment section while hitting the place order button. Here it's allowing to proceed the payment.
    Kindly do the needful. It's pretty urgent.

@magento-engcom-team
Copy link
Contributor

magento-engcom-team commented Feb 21, 2018

Hi @bka. Thank you for your report.
The issue has been fixed in #13543 by @hostep in 2.1-develop branch
Related commit(s):

The fix will be available with the upcoming 2.1.13 release.

@magento-engcom-team magento-engcom-team added Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in develop labels Feb 21, 2018
@codecodeio
Copy link

codecodeio commented Jun 25, 2021

I see this issue in Magento 2.3.5-p2. However, it only occurs when the customer has multiple stored payment methods. Each payment method agreement checkbox gets the same id making all but the first checkbox unclickable. We are using Braintree. Altering the file below with the code below in the function "getCheckboxId" resolves the issue.

File: app/code/Fss/Checkout/view/frontend/web/js/view/checkout-agreements.js

Suggested Change:
paymentMethodName = paymentMethodRenderer.index ? paymentMethodRenderer.index : '';
if(!paymentMethodName){
// item looks like this: {title: "Check / Money order", method: "checkmo"}
paymentMethodName = paymentMethodRenderer.item ? paymentMethodRenderer.item.method : '';
}

Screen Shot 2021-06-25 at 9 34 42 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Checkout Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Release Line: 2.1
Projects
None yet
Development

No branches or pull requests