Disable 'enter' submission on extend page

I've created a patch which introduces a very simple new behavior in system.modules.js which prevents the form from being submitted when the enter key is pressed anywhere inside of it.

It's documented but still very concise, I hope I did everything correctly. :)

This is more of a feature request, also un-assigning it.

Did a review before I realized this was a feature request. Normally enter should submit a form, but I can see in this case that it doesn't make sense for enter to submit the module list form. So, the only way someone could accidentally use 'enter' to submit the module list is if they make changes to a checkbox and then go to the search form and hit 'enter'. Even then, it's not a big deal because the page tells you which modules were enabled.

However, if this change ever happened, maybe if the "Save Configuration" button is highlighted, the submit should be allowed.

I'll check this.

I've rerolled the patch for it to work with the latest changes.

I'll also work on the highlighted stuff mentioned in #4

I've updated the patch to handle focus on the submit button.

Please review.

I disagree with adding this change. I find it extremely useful to be able to submit forms with the enter key and I typically find that apps should not mess with default browser behavior if possible. I don't see any arguments on this thread that make me think this form should receive special treatment in this regard.

I also want to point out that this suggestion has accessibility concerns. If you have to use your mouse to submit the form that is a big accessibility concern (I realize that #7 mitigates that concern to some extent). Yet another reason why we shouldn't meddle with browser behavior.

Accessibility concerns aside (for the time being)

I would argue that the module (extend) page requires this to keep from inadvertently installing modules.
This should also be an issue on the permissions page.

I can't see how this will cause an accessibility problem. In my testing of the patch in #7 it wasn't a problem. Just had to navigate to the bullet and hit enter while hovering focusing over the "Save configuration" button.

@mgifford, what do you mean by "hovering"? If you're talking about a mouse, then that's an accessibility problem. If you're saying that you need to tab to the "Save configuration" button before you can submit the form then that could be a giant accessibility problem. There are a lot of checkboxes on that page and if someone had a disability that prevented them from quickly tabbing (such as if you use a mouth stick to hit the tab key) that could mean hundreds of tab key presses before you can submit the form. Again, I just don't think that we should mess with default browser behavior. Why is this form different than say the node add form? Why is accidental submission more of a problem on this form than it would be on any other form?

I just edited this for clarification. No, I wasn't talking about using a mouse. This can still all be done with only a keyboard at least as I can see it now. There are pages like this & the permissions page that will be a pain for someone tabbing through the site as there is yet no pattern to allow keyboard only users to skip to the submit button. There isn't any way around this at this stage.

Not altering the browsers default behavior is a good argument against this patch.

I think it's different than the node add form simply as it can be so very, very long in comparison.

I think a good solution to this problem is to have a confirmation page when enabling modules/setting permissions.

yeah, maybe a confirmation page would solve all these issues more cleanly.

It could be a nice usability improvement to present a screen that summarizes the changes and seeks confirmation. This is becoming less of a novice issue though.

Probably, we should leave the page alone (at least in Core.) We shouldn't mess with the default behavior of the form. Someone might be using the page during a deployment and a confirmation would be annoying. I was originally less against the change because I could see someone typing in the search box and hitting enter, but the page is already permission protected.

I would leave it alone in 7, but in 8 I don't think we should be worried about breaking auto deployments.

I bring this up because I have enabled the wrong module by mistake (by pressing enter) and not all modules have proper uninstall hooks.

Could we disable the enter key only in the search box? Is that the real issue here? since that filter is client-side, I don't see any issues with disabling the enter key in the search box only. Actually, now that I think about it, maybe the search box just needs to be moved out of the form and into a separate form. I don't know the exact best practices on that, but I think that might address all the concerns or use the confirmation dialog.

I think this will be dealt with in #2056089: UI problems on the Modules/Extend page.

The other issue seems to be focused on the search form functionality. My concern is for the module enable form.

If a user checks a box to enable a module, then they have that form focused. Then if they hit enter, then the form submits and the modules are enabled.

A module enable confirm screen would do the trick. My preference is to disable the enter key, but that generally isn't considered a UX best practice.

I updated the issue body.