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

Please fix select_tag in 3.2.8 #7404

Closed
Sega100500 opened this issue Aug 21, 2012 · 8 comments · Fixed by #7410
Closed

Please fix select_tag in 3.2.8 #7404

Sega100500 opened this issue Aug 21, 2012 · 8 comments · Fixed by #7410

Comments

@Sega100500
Copy link

Hi!

In 3.2.7 this code was possible if 'auto_models_for_select' is nil :
<%= select_tag :model, auto_models_for_select, :include_blank => true,
:disabled => auto_models_for_select.nil? %>

In 3.2.8 this should be :
<%= select_tag :model, auto_models_for_select || '', :include_blank =>
true, :disabled => auto_models_for_select.nil? %>

If second parameter is nil it raise an exception.

Please, return back that it was possible to transfer nil of second
parameter - options.

@carlosantoniodasilva
Copy link
Member

What exception is raised? What's the stack trace of the exception? Please send as much info as you can, thanks.

@Sega100500
Copy link
Author

TypeError in Application#index

Showing (...) where line #32 raised:

can't convert nil into String

...

activesupport (3.2.8) lib/active_support/core_ext/string/output_safety.rb:114:in concat' activesupport (3.2.8) lib/active_support/core_ext/string/output_safety.rb:114:insafe_concat'
actionpack (3.2.8) lib/action_view/helpers/form_tag_helper.rb:125:in `select_tag'

@nashby
Copy link
Contributor

nashby commented Aug 21, 2012

@carlosantoniodasilva looks like we need to change nil to empty string here: https://github.com/rails/rails/blob/master/actionpack/lib/action_view/helpers/form_tag_helper.rb#L120

or make safe_concat to accept nil

@carlosantoniodasilva
Copy link
Member

Seems like it worked "by chance" so far:

>> ''.html_safe + nil
=> ""

And here's the culprit: a37c474. I think that ensuring option_tags is not empty should be enough, right?

options_tags ||= ''

@nashby
Copy link
Contributor

nashby commented Aug 21, 2012

yeah, this should work. Or:

def select_tag(name, option_tags = '', options = {})

?

@rafaelfranca
Copy link
Member

@nashby this will not work in the case that the second argument is nil. We will have to ensure that option_tags is not nil.

@nashby
Copy link
Contributor

nashby commented Aug 21, 2012

@rafaelfranca right.

@Sega100500
Copy link
Author

just add:
options_tags ||= ''

before work with options_tags in select_tag method

rafaelfranca added a commit that referenced this issue Aug 21, 2012
option_tags coerced to "" instead of nil

Closes #7404
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants