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

Nested model-backed form content (fields_for) gets html escaped #2

Open
johnlane opened this issue Oct 2, 2015 · 7 comments
Open

Comments

@johnlane
Copy link

johnlane commented Oct 2, 2015

As discussed on Gitter, I have this problem where nested fields_for are escaped when rendered. I've been doing some research and conclude that it happens

  • only if the form is rendering a model object
  • when the nested content is concatenated to the form output buffer
  • because a different concatenate << method is executed

TL;DR For a temporary workaround apply $ sed -i -e '25s:):).html_safe:' $(bundle show cells-erb)/lib/cell/erb/template.rb to cells-erb-0.0.6.


When the form is a model object the concat method in activesupport-4.2.4/lib/active_support/core_ext/string/output_safety.rb#184 is used:

   184:     def concat(value)
=> 185:       super(html_escape_interpolated_argument(value))
   186:     end
  • the receiving buffer self is a ActiveSupport::SafeBuffer
  • the concatenated buffer value is a String

When the form has no model it uses cells-4.0.2/lib/cell/view_model.rb#163

   163:       def <<(string)
=> 164:         super
   165:       end
  • the receiving buffer self is a Cell::ViewModel::OutputBuffer
  • the concatenated buffer string is a String

A solution to this problem is to apply .html_safe to the output buffer in cells-erb-0.0.6/lib/cell/erb/template.r:

 22     # this is capture copied from AV:::CaptureHelper without doing escaping.
 23     def capture(*args)
 24       value = nil
 25       buffer = with_output_buffer { value = yield(*args) }
 26 
 27       return buffer.to_s if buffer.size > 0                                       
 28       value # this applies for "Beachparty" string-only statements.
 29     end

Replace line with

25      buffer = with_output_buffer { value = yield(*args).html_safe }

I do not understand the code well enough to know if that is the right solution but it does solve the problem for my use-case.

johnlane added a commit to johnlane/cells-erb that referenced this issue Oct 2, 2015
johnlane added a commit to johnlane/cells-erb that referenced this issue Feb 3, 2016
@asia653
Copy link

asia653 commented May 26, 2016

Hi perhaps submit a PR for this fix? I am running to this as well
@apotonick will this be an acceptable patch?

@johnlane
Copy link
Author

I never submitted a PR for this because my work-around uses html_safe which is rails-specific and cells-erb is not (hopefully I have that right). At some point I guess @apotonick will comment on this issue and suggest what is right. If he wants this then I will PR it, but I suspect there will be a better solution that isn't rails-specific.

@p12y
Copy link

p12y commented Nov 19, 2016

Any ideas for alternatives to this workaround? It fixes the rendering of the fields_for but causes some strange rendering issues for the rest of my cells.

@apotonick
Copy link
Member

Use Erbse master and it should all work!

@p12y
Copy link

p12y commented Nov 19, 2016

I'm using erbse master and still getting the same problem. After installing the latest commit and adding html_safe to the capture method on my end, it's solved the problem for me.

def capture(*args)
  yield(*args).html_safe
end

@lukeasrodgers
Copy link

@PeteTyldesley have you tried something like this?

In your cell:

def my_fields_for(form, sym, &block)
  form.fields_for sym do |subfields|
    (yield subfields).html_safe
  end
end

and in your view call my_fields_for instead of fields_for. This works for me (though I'm using simple_form gem instead of the plain actionview helpers).

Maybe not ideal, but at least you don't have to use the shotgun approach of always returning html_safe strings from capture. From my reading of the erbse README, some sort of manual integration like this will be necessary.

@p12y
Copy link

p12y commented Dec 5, 2016

@lukeasrodgers Thanks, that is a much better approach. You are totally right, there's no need for such a brute force method as the one I mentioned!

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

No branches or pull requests

5 participants