Improve semantics of orphaned form elements such as buttons

Disclosure: I work for AgileBits, makers of 1Password.

I was diagnosing an issue with 1Password failing to offer to save user’s credentials when clicking the “Log In” button in the modal sign in dialog. The main issue turned out to be that the “Log In” <button> is not a child of the <form> that it controls and does not have a form attribute set. With these kinds of orphaned elements, 1Password is much more reserved about its interpretation of the user’s clicks.

I arrived at a solution that will work around this in the 1Password extension, but it would be more semantic if Discourse’s buttons were either children of their <form> elements or had explicit relationship set up via the form attribute.

It would also be appropriate to explicitly set the type attribute of this button to “submit”. Even though this is the default attribute value, apps like 1Password look for an explicit attribute because lots of sites use buttons with no type attribute for things that do not have a submit behavior.

I got as far as tracking this down to login.hbs:43 and was going to make a PR to add the form="login-form" attribute, but had some issues getting the dev environment set up and ran out of time so I punted to creating a bug report. :frowning:

Thanks!


Jamie Phelps
Code Wrangler @ AgileBits
Fort Worth, Texas

11 Likes

I’ll fix it. I :heart: 1Password. Thanks for taking the time to describe both the issue and the solution.

9 Likes

Thanks so much! I really appreciate the quick response. :grinning:

https://github.com/discourse/discourse/commit/f2219138e17a2df704c12885b6c832e21600a54b

8 Likes

Had to revert my commit because of this bug. This requires a larger refactoring…

https://meta.discourse.org/t/twitter-popup-on-enter/73048

4 Likes

Weird it is much more logical for the form buttons to be inside the form…

3 Likes

When you are back next week @zogstrip I would like to resume work on this one.

1 Like

I’ve added the form attribute and set the type attribute to “submit” to the Log In button.

https://github.com/discourse/discourse/commit/8c91d418ddc240cf6f1faf3561c140b1bdbce130

@jxpx777 is it enough or do I need to make Log In a child element of the <form>?

3 Likes

Any further comments on this @jxpx777 — are we good now, does it work?

This topic was automatically closed after 6 days. New replies are no longer allowed.