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.