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

Already on GitHub? Sign in to your account

Invalid HTML for collection_check_boxes #215

Closed
javierv opened this Issue Mar 15, 2011 · 10 comments

Comments

Projects
None yet
5 participants

javierv commented Mar 15, 2011

Right now, collection_check_boxes generates the following HTML for each checkbox:

<label class="collection_check_boxes" for="post_tag_ids_697">
  <input name="post[tag_ids][]" type="hidden" value="" />
  <input class="check_boxes optional" id="post_tag_ids_697" name="post[tag_ids][]" type="checkbox" value="697" />
  Label text
</label>

This goes against two HTML rules:

  • The label element may contain at most one input, button, select, textarea, or keygen descendant.
  • Any input descendant of a label element with a for attribute must have an ID value that matches that for attribute.

Of course, the second one is a consequence of the first one.

Here's a suggested test case: https://gist.github.com/871153

I haven't been able to come up with a patch yet. Apparently, Rails' check_box method doesn't offer the option not to generate the hidden field, which I guess would be the cleanest solution.

Another solution would be to put the inputs outside the label and maybe wrap them in a div, but I don't think that was your intention :-). Does formtastic do that?

Thanks.

Collaborator

carlosantoniodasilva commented Mar 16, 2011

Ok I agree this might be an issue, but both HTML4 and XHTML work fine. It just happens using HTML5 doctype as far as I know, which is pretty interesting actually.

Anyway, the only solution would be to move the checkbox outside the label as you said, instead of having it as a wrapper, because the hidden input is added automatically by Rails and is required to mitigate an issue with unchecked checkboxes. The thing is that it was like this before, and we changed due to someone's request if I remember correctly. So it gets hard to decide which way to go :).

javierv commented Mar 16, 2011

Yeah, simple things are complicated :-).

Interesting comment. I didn't know it happened only with HTML5. It makes sense to me, since labels must be associated with one and only one input.

Anyway, I've no idea about what the best solution is :). If you want to keep the input inside the label, using check_box_tag would be unneccessarily messy, and adding a method doing the same thing as check_box does but without adding the hidden field doesn't look any better.

For this reason I can not use jquery-ui's button widget (:as => :check_boxes) with simple_form...

Collaborator

rafaelfranca commented Mar 30, 2011

This behavior has added by @josevalim on this commit: 64ddad0

Maybe he knows why it changed.

Owner

josevalim commented Mar 30, 2011

I changed it because would now have a wrapper around the input + text, making it easy to customize if you want to have both together. It seems the best solution here is to move the hidden input out of the label, although this implies we won't be able to use the rails wrapper anymore.

Collaborator

carlosantoniodasilva commented Mar 30, 2011

Yeah, this would be a solution, but it for sure does not make me happy =(.

Collaborator

rafaelfranca commented Apr 6, 2011

Guys, if the reason to use the input inside the label is to have a wrapper, what do you think of change the label to a div, and use the label aside of input?

Collaborator

carlosantoniodasilva commented Apr 6, 2011

For that you could use the :item_wrapper_tag option that was added to SimpleForm, so I think we can just use check_box + label instead of having the label as wrapper. Seems the simplest way to fix this imo.

Collaborator

rafaelfranca commented Apr 6, 2011

I'll try to implement this

Collaborator

rafaelfranca commented Apr 6, 2011

Now it's fixed on master. Thanks for reporting.

@carlosantoniodasilva carlosantoniodasilva added a commit that referenced this issue Jan 27, 2012

@carlosantoniodasilva carlosantoniodasilva Do not generate hidden check box field when using nested boolean style
It is considered invalid markup in HTML5. This will only work in
Rails > 3.2.1 (not released at this time). If you really care that much
about this, please use Rails 3-2-stable branch for now, that contains a
backported fix from master.

Backport commit: rails/rails@2e5ec3b
More info in #215
0b34519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment