content_for is not working as described when content is not set

[killthekitten]

Following this example from documentation, content_for can be used as a part of boolean statement:

...
# <tt>content_for</tt>, however, can also be used in helper modules.
#
#   module StorageHelper
#     def stored_content
#       content_for(:storage) || "Your storage is empty"
#     end
#   end
...

But it does not behave as described. content_for output is wrapped with ActiveSupport::SafeBuffer which casts nil value to an empty string instead of just returning nil.

To be clear, I expect content_for to return nil if no content is set. Here is my case:

# If content for header was passed, render it, else render default partial 
= content_for(:header) || render 'partials/header'

And it does not work as expected. My current workaround:

= content_for?(:header) ? content_for(:header) : render 'partials/header'

Do we need to update example? Or is it a bug?

[sikachu]

If that's define in the doc like that, I think that's a bug/regression. Good finding!

Would you mind submitting a failing test case, or even better a fix for this? Thank!

[senny]

I would also trust the documentation if this was added to the docs it once was a use-case.

@killthekitten I could take a look and submit a PR to solve the problem. Let me know if you are not working on it.

[killthekitten]

@sikachu sure, I'll do it. But currently I have no idea how to make actual fix. @senny any thoughts?

[senny]

@killthekitten I did not look into the source yet... but the problem seems straight forward. You could look what commit added the example in the documentation and take a look at how the code looked back then to ensure if it worked once. Then write a failing test-case and as last step change the code so that it passes your newly written test and all others on AP/AV.

[sikachu]

Yeah, I'm pretty sure it was overlooked when we introduced SafeBuffer.

[killthekitten]

Sad, this PR breaks many tests. Didn't run the full suite before commit.

[killthekitten]

This commit is better, but I don't like the code complication that it brings. What do you think? /cc @sikachu @senny

[sadfuzzy]

Hey, it is broken again in 3.2.19 :) and defaults to an empty string

[killthekitten]

@sadfuzzy and did you check if tests are also failing (probably they don't, but can you provide us with a failing case?)?