This project is archived and is in readonly mode.
FormHelper's auto_index should create more "friendly" ids or revert back to id_before_type_cast
Reported by Brian Johnson | June 26th, 2009 @ 03:54 AM | in 3.x
sanitized_object_name allows . and : which are perfectly legal according to the specs, but still cause issues with jQuery in particular and I think some browsers as well. It seems to actually complicate things, the to_param is meant for more user friendly urls, I don't see why it would apply to an html tag id where it seems to me the id field would be much more appropriate. I found this ticket http://dev.rubyonrails.org/ticket/9994 but no discussion about it. Is anyone actually using this that wants to_param? If so, why do you prefer that over id?
Comments and changes to this ticket
-
Brian Johnson June 26th, 2009 @ 03:56 AM
This ticket is a perfect example of why to_param seems like the wrong thing to do. Imagine an id that is ticket_2840-formhelpers-auto_index-should-create-more-friendly-ids-or-revert-back-to-id_before_type_cast_comment
Doesn't that seem a little ridiculous? -
Yehuda Katz (wycats) July 2nd, 2009 @ 07:20 PM
- State changed from “new” to “wontfix”
It seems like changing this at this point would potentially break people's JavaScript code that rely on the existing behavior for marginal benefit. Can you clarify a bit more what the significant improvement would be that justifies the breakage? Can you provide some examples?
I'm marking this as wontfix but would be open to revisiting this with some more compelling examples.
-
Brian Johnson July 2nd, 2009 @ 07:32 PM
what about at least removing . and : from the allowed characters? Technically they're legal, but they do cause issues. I started this ticket because I had a to_param method with a . in it that broke my javascript code and css selectors in Firefox and then I thought it would be better to just use the ID. If your to_param method has a dot in it, css would interpret that as a class selector.
-
Brian Johnson July 2nd, 2009 @ 07:37 PM
another option would be an option :index => :id_before_type_cast where the symbol would be any method you want to run as the index parameter
-
Brian Johnson July 2nd, 2009 @ 07:44 PM
if you think that would work, I can come up with a patch, but I don't want to spend time on something that isn't going to be used.
-
Yehuda Katz (wycats) July 2nd, 2009 @ 08:29 PM
- Assigned user set to “Yehuda Katz (wycats)”
@brian hmmm... I just wanted to point out that at least the jQuery selector engine supports escaping, so you could do $("#myid\.id") or $("#myid\:id") just fine. I'll give this a bit more thought after lunch.
-
-
Brian Johnson July 2nd, 2009 @ 10:00 PM
I'll try that again
<html> <style> #test.dot { color: red; } </style> <body> <div id="test.dot">test</div> </body> </html> -
-
Brian Johnson July 2nd, 2009 @ 10:15 PM
so I guess maybe there is no real issue with . or : in general, but it doesn't change the fact that it would be nice to have the option to use the id instead. I think the :index => :method_name is a good way to go.
-
Yehuda Katz (wycats) July 2nd, 2009 @ 10:17 PM
- State changed from “wontfix” to “hold”
@brian If you can make a patch that only adds a net line or two, I'm game!
-
Yehuda Katz (wycats) July 2nd, 2009 @ 10:17 PM
- State changed from “hold” to “incomplete”
-
Brian Johnson July 2nd, 2009 @ 11:30 PM
ok, I needed two lines to cover nested forms. I used method(method_name).call instead of send because the test was failing on the Structs in the tests, not sure if there is a better way to deal with that.
-
-
Rohit Arondekar October 9th, 2010 @ 04:02 AM
- State changed from “incomplete” to “stale”
- Importance changed from “” to “”
Marking ticket as stale. If this is still an issue please leave a comment with suggested changes, creating a patch with tests, rebasing an existing patch or just confirming the issue on a latest release or master/branches.
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>
Brian Johnson
Jeremy Kemper
Rohit Arondekar
Yehuda Katz (wycats)