Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

form input id collisions #1001

Open
manveru opened this issue Jan 23, 2024 · 3 comments
Open

form input id collisions #1001

manveru opened this issue Jan 23, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@manveru
Copy link
Contributor

manveru commented Jan 23, 2024

When there are more forms on a single page, the id attributes is not unique for each label/input.
That is a problem for accessibility and usability, since now clicking the label may activate completely unrelated input.

One can work around this by manually setting the id for each tag for now, but it would be awesome if there was a way to do this automatically. Maybe generating a random prefix for each form_for instance?

@robacarp
Copy link
Contributor

@manveru can you give a more concrete example of what you're facing?

@manveru
Copy link
Contributor Author

manveru commented Jan 23, 2024

My actual usage is more complicated, but the problem is quite easy to reproduce on a tiny scale.
Let's say we have something like this:

[user1, user2].each do |user|
  form_for User::Update do
    mount Users::FormFields, SaveUser.new(user)
    submit r("save").t, data_disable_with: r("saving").t, class: "button is-link"
  end
end

This will generate fields like these (slightly different tag structure since I adapted it for Bulma, but the tag_defaults and tag_builder are the same:


<form action="/users/1" method="post">
  <input type="hidden" name="_csrf" value="xxx">
  <input type="hidden" name="_method" value="put">
  <div class="field">
    <label for="user_name">Name</label>
    <div class="control">
      <input type="text" id="user_name" name="user:name" value="Alpha" class="input" autofocus="true">
    </div>
  </div>
  <input type="submit" value="Update" data-disable-with="Updating..." class="button is-link"></form>

<form action="/users/2" method="post">
  <input type="hidden" name="_csrf" value="xxx">
  <input type="hidden" name="_method" value="put">
  <div class="field">
    <label for="user_name">Name</label>
    <div class="control">
      <input type="text" id="user_name" name="user:name" value="Beta" class="input" autofocus="true">
    </div>
  </div>
  <input type="submit" value="Update" data-disable-with="Updating..." class="button is-link"></form>

Now the issue is that the label for doesn't have a unique id to point to anymore, since both inputs have the id of user_name.

This not only violates the HTML spec, which doesn't allow duplicated id attribute values, but it also messes up the association of a label with its input. That's particularly problematic when it comes to the checkbox and radio types, since users often like to click on the label instead of the tiny input element.

I hope this clarifies the issue a bit.

@jwoertink jwoertink added the bug Something isn't working label Jan 23, 2024
@jwoertink
Copy link
Member

I was thinking about this today... Would appending the model's id to the html id be sufficient? So you'd have id="user_name_1" or id="user_name_afc8-9bsd-39fh-93hf", for example. Obviously you could still override the id attribute if you need, but that would be a quick fix. The only thing I don't like is I'm not a fan of exposing user IDs in my apps.

The other option I thought of is some sort of page level context store for how many times a specific ID gets generated with some count. Then you end up having something like Lucky::Page::GLOBAL_HTML_ID_COUNTS which might have {"user_name" => 2} and that gets pulled in. I'm not sure if it would work doing it at a page instance level if you're pushing forms off to components like

[user1, user2].each do |user|
  mount UserForm, user: user
end

But doing it on a global scale means that if you mount this user form on several pages, it's possible you might have a field that's like id="user_name_6" because the count would increment each time the field was mounted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants