Improve semantics of orphaned form elements such as buttons

Contribute UX

AI-generated summary

Improving Semantics of Orphaned Form Elements

jxpx777 from AgileBits reported an issue with 1Password failing to save user credentials when clicking the “Log In” button in Discourse’s modal sign-in dialog. The problem was that the <button> was not a child of the <form> it controlled and lacked a form attribute. jxpx777 suggested making the button a child of the form or adding the form attribute to resolve the issue.

zogstrip agreed to fix the issue and initially committed a change, but later reverted it due to a bug requiring a larger refactoring. codinghorror pointed out the logic of having form buttons inside the form.

After further discussion, zogstrip added the form attribute and set the type attribute to “submit” for the “Log In” button. codinghorror asked for confirmation from jxpx777 if this solution was sufficient or if the button needed to be a child element of the <form>.

1

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 лайков
2

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

9 лайков
3

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

4

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

8 лайков
5

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 лайка
6

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

3 лайка
7

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

1 лайк
9

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 лайка
10

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

11

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