This project is archived and is in readonly mode.
InstanceTag to_label_tag id attribute
Reported by Di Marcello | May 17th, 2009 @ 02:24 PM | in 2.x
InstanceTag method to_label_tag produces the wrong for attribute when an id options is given.
The solution is to add the marked line in to_label_tag:
name_and_id = options.dup
+ name_and_id["id"] = name_and_id.delete("for")
The attachment adds this line and a test (could not run the test, cause i get a Debugger:Module not found error, its the first time i did this, so i dont know how to fix this)
Comments and changes to this ticket
-
Di Marcello May 17th, 2009 @ 02:25 PM
somehow something went wrong in my ticket body.
it should look like this:name_and_id = options.dup
+ name_and_id["id"] = name_and_id.delete("for") -
Di Marcello May 17th, 2009 @ 02:28 PM
damnit!!!! my lines get messed up.
the plus sign was the mark. So the line I added was only:name_and_id["id"] = name_and_id.delete("for") -
Di Marcello June 3rd, 2009 @ 11:49 AM
Why doesnt anyone picks this up?
Did i do something wrong? it isnt such a big fix -
Matt Jones June 8th, 2009 @ 09:46 PM
The diff you've provided doesn't make sense against 2-3-stable, and the test doesn't appear likely to fail there either. What version of Rails did you encounter this problem on?
-
Di Marcello June 19th, 2009 @ 01:17 AM
on rails 2.3.2 the github master branche at the time of posting
-
Matt Duncan August 8th, 2009 @ 04:52 PM
- Tag changed from “actionpack, actionview, form_helper, instance_tag, label, to_label_tag” to “actionpack, actionview, bugmash, form_helper, instance_tag, label, to_label_tag”
Patch didn't apply cleanly for me so I re-created it and tweaked the tests for better coverage.
I've attached a patch.
-
Adam Keys August 8th, 2009 @ 05:26 PM
I've verified this applies cleanly to 2-3-stable. Test coverage looks good. +1
-
Josh Nichols August 9th, 2009 @ 08:48 PM
- Tag changed from “actionpack, actionview, bugmash, form_helper, instance_tag, label, to_label_tag” to “actionpack, actionview, bugmash, form_helper, instance_tag, label, to_label_tag, verified”
It's reasonable enough to want to add an ID to a label, so +1.
Verified Matt's patch applies to 2-3-stable, and works as advertised.
-
-
Elad Meidar August 9th, 2009 @ 09:36 PM
+1 verified, +1 patch applies on 2-3-stable and master, tests pass on both.
i can think of several cases where i'd like to have id on a label.
-
Tristan Dunn August 9th, 2009 @ 11:46 PM
+1
Verified the patch applies to master and 2-3-stable cleanly with passing tests.
-
Repository August 9th, 2009 @ 11:57 PM
(from [ba961250bd07a131329fb90cc998c1f28d5aac14]) Fixed to_label_tag to accept id attribute without changing for attribute [#2660 status:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/ba961250bd07a131329fb90cc998c1... -
Repository August 9th, 2009 @ 11:57 PM
(from [920ef4e6f3832ea1d3297e2e7e5f0f775dab69a8]) Fixed to_label_tag to accept id attribute without changing for attribute [#2660 status:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/920ef4e6f3832ea1d3297e2e7e5f0f... -
José Valim August 9th, 2009 @ 11:58 PM
- State changed from “new” to “resolved”
- Tag changed from “actionpack, actionview, bugmash, form_helper, instance_tag, label, to_label_tag, verified” to “actionpack, actionview, form_helper, instance_tag, label, to_label_tag, verified”
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
Adam Keys
José Valim
Josh Nichols
Attachments
Referenced by
-
2660
InstanceTag to_label_tag id attribute
(from [ba961250bd07a131329fb90cc998c1f28d5aac14])
Fixed t...
-
2660
InstanceTag to_label_tag id attribute
(from [920ef4e6f3832ea1d3297e2e7e5f0f775dab69a8])
Fixed t...