This project is archived and is in readonly mode.
sending :id => nil to form helpers does not remove the "id" html attribute
Reported by Jeff Dean | May 9th, 2010 @ 07:51 AM
I have several apps where I don't need the id for anything. Especially now that nested_attributes produce such long ids, I frequently want to omit the "id" html attribute to save bandwidth and rendering time. However, this is not currently possible:
text_field "foo", "bar", :id => nil # => <input type="text" id="foo_bar" name="foo[bar]" />
I plan to submit a patch momentarily.
Comments and changes to this ticket
-
Matt Jones May 9th, 2010 @ 04:58 PM
Interesting idea, but the stated goal ("save bandwidth") smells an awful lot like premature optimization. Are you really generating enough deeply-nested fields to save more than a few bytes?
This also seems likely to cause more complication for anybody who is trying to generate
text_field
calls programmatically:def some_text_field_helper(name, id=nil) text_field some_function_of_name(name), 'something', :id => id end
The above is going to be trickier to code with this patch than without.
-
Dan Pickett May 9th, 2010 @ 05:48 PM
- Tag changed from form_helper, patch to bugmash, form_helper, patch
-
Jeff Dean May 9th, 2010 @ 07:32 PM
For me the biggest issue is inconsistency. If I pass :class => nil to a text_field, no class attribute is emitted. If I pass :id => nil, the default id is emitted. On that basis alone (principle of least surprise), I think current behavior is a bug.
I've encountered a number of other places where having no ids is beneficial - for example when I have the same form in multiple places on the same page (think the "new task" form within each task list in Basecamp) and I want the HTML to validate. In that case I'm forced to use an index, which I'm only doing for the purpose of getting valid HTML because Rails won't respect :id => nil.
Matt - examples like the one you gave would be trivially more complex, but I think that's a totally acceptable price to pay for consistency across attributes.
-
Josh Kalderimis May 10th, 2010 @ 11:23 AM
+1
Based on the inconsistency of :class and :id behavior
Is it only text_field, text_area and check_box which are affected?
-
Wijnand Wiersma May 15th, 2010 @ 12:03 PM
+1
but I think this should affect all input types.
Just a quick test reveals it doesn't work for radio buttons. -
Wijnand Wiersma May 15th, 2010 @ 12:03 PM
- no changes were found...
-
Wijnand Wiersma May 15th, 2010 @ 01:23 PM
My previous comment (and sorry for the empty one) was incorrect.
Using f.radio_button it does skip the id, I did a quick test using radio_button_tag so that explains my incorrect observation.The patch seems to be pretty complete to me and works fine.
-
Casey Dreier May 15th, 2010 @ 08:16 PM
+1 Patch applies just fine. Verified.
I made a tiny addition of adding a test for verifying that the id is not generated for radio_boxes.
Note that this patch as implemented does not currently allow you to specify :id => nil for select boxes. Is this something that should be added for consistency?
-
Jeff Dean May 15th, 2010 @ 09:15 PM
I think it should be consistent. I'm adding support for that now, and will re-post a patch.
-
Rizwan Reza May 15th, 2010 @ 09:15 PM
- Tag changed from bugmash, form_helper, patch to bugmash-review, form_helper, patch
-
Jeff Dean May 15th, 2010 @ 09:26 PM
Thanks Casey. "select" actually did work as expected, and the syntax is:
select("post", "secret", [], {}, "id" => nil)
I've reattached the patch with an extra test case to support this, but I lost the "Sign-Off" by Casey because I didn't know how to keep it.
-
Repository May 15th, 2010 @ 09:46 PM
- State changed from new to resolved
(from [6617d0189377a2f820c8f948589bb2d4a91155af]) Sending :id => nil to form helpers now properly omits the "id" html element [#4559 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/6617d0189377a2f820c8f948589bb2... -
Jeff Dean May 15th, 2010 @ 10:13 PM
Awesome - thanks José!
I just realized that I used the arity-2 form of fetch, instead of the block format. Here is a patch that will make this a bit more performant. It's a straight refactoring, and the behavior doesn't change so I don't see the need to change the tests.
-
Rizwan Reza May 16th, 2010 @ 02:07 AM
- Tag changed from bugmash-review, form_helper, patch to form_helper, patch
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Tags
Referenced by
- 4559 sending :id => nil to form helpers does not remove the "id" html attribute (from [6617d0189377a2f820c8f948589bb2d4a91155af]) Sending...
- 4613 ActionPack tests have errors and failures I have fixed 3 errors and 1 failure and will submit a pat...
- 4613 ActionPack tests have errors and failures I have fixed 3 errors and 1 failure and will submit a pat...