It seems having a required field in the registration form causes the login of an existing user to be blocked unless they fill out the required field at cart checkout as anonymous user.
steps to reproduce:
1. Add a required field to the default Drupal account settings (/admin/config/people/accounts/fields)
2 Make sure "allow registration" is active on the Login pane of the checkout flow of your store (/admin/commerce/config/checkout-flows)
3. Make sure you're not logged in, then add a product to your cart as an anonymous user
4. Go to cart->checkout
5. The Login page should now show, asking you to either log in or register as a new customer. The new customer form should contain the required field created in step 1
6. Attempt to log in as an existing user
result:
The browser blocks the submit of the form, because the field has the attribute required="required" (see attached screenshot)
expected:
The login form should still be submitted and the user logged in, even though the registration form shown on the same page contains a required field.
This bug was introduced since release 2.12, probably due to feature https://www.drupal.org/node/2921312 being merged. The problem I guess is that all the inputs on this page are wrapped in one single HTML form element, thus clicking any of the submit buttons on the page will cause the browser to check all inputs for the "required" attribute.
The solution probably is to wrap each subform into their own form element, so they are independent also from the browsers point of view?
Comment | File | Size | Author |
---|---|---|---|
#16 | commerce-3035519-login_validation-16.patch | 3.38 KB | czigor |
| |||
#16 | commerce-3035519-login_validation-ONLY_TEST-16.patch | 2.2 KB | czigor |
Screenshot 2019-02-25 at 13.01.26.png | 93.86 KB | jwwj |
Comments
Comment #2
jwwj CreditAttribution: jwwj as a volunteer commentedIf someone can point me in the direction where I should look into maybe splitting up the forms into their own separate html, then I could try to create a patch to see if it solves the issue. Theming in Drupal is still something of a black box for me, so I'm struggling with figuring out where these commerce forms are actually generated...
Comment #3
Martijn de WitMaybe this issue relates to this problem.
Comment #4
jwwj CreditAttribution: jwwj as a volunteer commentedI worked around this for now by having a custom FieldWidget that has it's own "required" validation, which allowed me to set the field as not required in the Drupal built in configuration settings and still have it enforced upon submit. Not pretty, but...
Still think the correct way to solve this would be to split the forms so they would all be wrapped in their own form tag...
Comment #5
Ice-D CreditAttribution: Ice-D commentedStruggling with the same issue. This should probably be fixed since it's not uncommon to have required fields in registration.
Comment #6
yonailo CreditAttribution: yonailo as a volunteer commentedSame problem here, any clues what would be the best way to solve this issue ?
Comment #7
jidrone CreditAttribution: jidrone as a volunteer commentedHere is a small patch for that.
Just to mention it was needed to set the #submit property for submit button as an empty array to be able to use #limit_validation_errors, see FormValidator::determineLimitValidationErrors
Comment #8
jidrone CreditAttribution: jidrone as a volunteer commentedSorry, I forgot to set the parent checkout pane as a dynamic value.
Comment #9
jwwj CreditAttribution: jwwj as a volunteer commentedThe patch in #8 seems to fix the original issue, thanks! Tested against Drupal 8.7.7 / Commerce 2.14
Comment #10
bojanz CreditAttribution: bojanz at Centarro commentedMarked #3087240: A profile with include register form causes validation errors on guest checkout as a duplicate.
Comment #11
bojanz CreditAttribution: bojanz at Centarro commentedFix makes sense.
Let's add a failing test as well. Adding tag.
Comment #12
czigor CreditAttribution: czigor at Centarro commentedComment #13
czigor CreditAttribution: czigor at Centarro commentedAdded the #limit_validation thing to the "Continue as Guest" submit button too.
Comment #14
bojanz CreditAttribution: bojanz at Centarro commentedThe fix is good, the test isn't.
We need to test the case from this issue, a required field added to the register form, and then ensuring that Login is possible.
Comment #15
czigor CreditAttribution: czigor at Centarro commentedThat's what the test is doing. The test was already adding a required field to the user register form so I did not have to change that. However, due to the login pane's configuration the user registration form did not appear on checkout. That's what was fixed by setting both the allow_registration and allow_guest_checkout settings to TRUE.
Comment #16
czigor CreditAttribution: czigor at Centarro commentedUsing the Login part of the pane in the test instead of the Guest checkout part.
The fix itself did not change.
Comment #17
bojanz CreditAttribution: bojanz at Centarro commentedComment #19
bojanz CreditAttribution: bojanz at Centarro commentedTweaked the #limit_validation_errors for login to not assume that there's only a single pane form parent, and committed.
Thank you jidrone and czigor!
Comment #21
MegaChriz CreditAttribution: MegaChriz at WebCoo commentedThis did broke a custom form alter on this form: I had added an extra field (email) to the guest fieldset and I noticed that:
@czigor (re #13)
I'm not sure if limit validation errors in the guest fieldset were also needed?