Skip to content

Loading…

fix block.arity will raise nil error #8640

Merged
merged 1 commit into from

2 participants

@jasl

content_tag_for will raise nil:NilClass when not given a block, cause content_tag_for_single_record will check block.arity == 0 but in this case "block" should be nil obviously.

@rafaelfranca
Ruby on Rails member

Could you add a test case?

@jasl

ok I‘ll do it later

@jasl

@rafaelfranca
I add test case
sorry in last commit I do something wrong
now content_tag_for without given block works as mentioned in document

@rafaelfranca rafaelfranca commented on the diff
actionpack/test/template/record_tag_helper_test.rb
@@ -81,6 +81,15 @@ def test_content_tag_for_collection
assert_dom_equal expected, actual
end
+ def test_content_tag_for_collection_without_given_block
+ post_1 = RecordTagPost.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true }
+ post_2 = RecordTagPost.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true }
+ expected = %(<li class="record_tag_post" id="record_tag_post_101"></li>\n<li class="record_tag_post" id="record_tag_post_102"></li>)
+ actual = content_tag_for(:li, [post_1, post_2])
+ assert_dom_equal expected, actual
+
@rafaelfranca Ruby on Rails member

Remove this blank line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rafaelfranca
Ruby on Rails member

Seems good. Could you add a CHANGELOG entry?

@rafaelfranca
Ruby on Rails member

And please squash your commits

@jasl

@rafaelfranca
what about now?
sry, this my first commit to a open source project.

@rafaelfranca rafaelfranca merged commit 204109e into rails:3-2-stable
@rafaelfranca
Ruby on Rails member

Very good. Thank you so much.

@rafaelfranca rafaelfranca added a commit that referenced this pull request
@rafaelfranca rafaelfranca Make content_tag_for work without block
This is version of #8640 for master
7e2ef18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
View
5 actionpack/CHANGELOG.md
@@ -1,5 +1,10 @@
## Rails 3.2.10 (unreleased) ##
+* Fix a bug in `ActionView::Helpers::RecordTagHelper::content_tag_for`
+ raise `nil:NilClass` without given a block.
+
+ *Jasl*
+
* Clear url helper methods when routes are reloaded by removing the methods
explicitly rather than just clearing the module because it didn't work
properly and could be the source of a memory leak.
View
4 actionpack/lib/action_view/helpers/record_tag_helper.rb
@@ -98,7 +98,9 @@ def content_tag_for_single_record(tag_name, record, prefix, options, &block)
options, prefix = prefix, nil if prefix.is_a?(Hash)
options = options ? options.dup : {}
options.merge!(:class => "#{dom_class(record, prefix)} #{options[:class]}".strip, :id => dom_id(record, prefix))
- if block.arity == 0
+ if !block_given?
+ content_tag(tag_name, "", options)
+ elsif block.arity == 0
content_tag(tag_name, capture(&block), options)
else
content_tag(tag_name, capture(record, &block), options)
View
8 actionpack/test/template/record_tag_helper_test.rb
@@ -81,6 +81,14 @@ def test_content_tag_for_collection
assert_dom_equal expected, actual
end
+ def test_content_tag_for_collection_without_given_block
+ post_1 = RecordTagPost.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true }
+ post_2 = RecordTagPost.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true }
+ expected = %(<li class="record_tag_post" id="record_tag_post_101"></li>\n<li class="record_tag_post" id="record_tag_post_102"></li>)
+ actual = content_tag_for(:li, [post_1, post_2])
+ assert_dom_equal expected, actual
+ end
@rafaelfranca Ruby on Rails member

Remove this blank line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
def test_div_for_collection
post_1 = RecordTagPost.new.tap { |post| post.id = 101; post.body = "Hello!"; post.persisted = true }
post_2 = RecordTagPost.new.tap { |post| post.id = 102; post.body = "World!"; post.persisted = true }
Something went wrong with that request. Please try again.