Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.If the label field is not required for example, the machine_name will stay hidden with an inline style of "display: none;".
In Chrome this leads to the situation that if you submit the form you don't get an HTML5 form validation error, but the also the form will not submit, because the browser will throw an error saying "An invalid form control with name='id' is not focusable." There is no information for the user why the form is not submittable.
To fix this:
- The machine name filed should only be "visually hidden"
- if the field is invalid (empty), the field including the validation error should be shown.
| Task | Novice task? | Contributor instructions | Complete? |
|---|---|---|---|
| Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards | Instructions |
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | interdiff_2488884-1-11.txt | 995 bytes | pixelmord |
| #11 | drupal-core-machine-name-frontend-validation-2488884-11.patch | 1.27 KB | pixelmord |
| #5 | machine-name.gif | 3.31 MB | tim.plunkett |
| #1 | drupal-core-machine-name-frontend-validation-2488884-1.patch | 1.54 KB | pixelmord |
Comments
Comment #1
pixelmord CreditAttribution: pixelmord at Wunder commentedHere is a patch that sets the visually-hidden class instead of using jQuery.hide() and adds an event listener to the machine name input to show its container in case it is invalid (empty in this case).
Comment #2
pixelmord CreditAttribution: pixelmord at Wunder commentedReady to test and review
Comment #3
pixelmord CreditAttribution: pixelmord at Wunder commentedComment #4
jmarkel CreditAttribution: jmarkel as a volunteer commentedComment #5
tim.plunkettI manually tested it and it worked great!
We can't really test this, so RTBC
Comment #6
nod_Agreed with the patch but the UX of that admin page is pretty bad, if the machine name is required the label should be too really. Or the form rearranged at least.
Comment #7
tim.plunkettI did fix that immediately after :)
http://cgit.drupalcode.org/page_manager/commit/?id=e2cda93
But I still think the patch is reasonable to go in.
Comment #8
dsnopekRTBC +1!
Comment #10
droplet CreditAttribution: droplet commentedI agreed it still reasonable to go in. But instead of a new `invalidHandler`, it should call `clickEditHandler` to end up auto-generated machine name.
Comment #11
pixelmord CreditAttribution: pixelmord at Wunder commented@droplet: Good idea, will save some lines of code.
I had to remove the e.preventDefault() in clickEditHandler() handler though, because it will prevent the machine name field to get focus. However this is unproblematic, because the we do not need to prevent the default anyways because the other listener is bound to the span.admin-link.
Comment #12
pixelmord CreditAttribution: pixelmord at Wunder commentedPlease review the modified patch
Comment #13
droplet CreditAttribution: droplet commentedGreat! Much better.
Comment #14
webchickThat #5 gif is AWESOME! :D Much better.
Committed and pushed to 8.0.x. Thanks!