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

Problem with escaped fields_for / simple_fields_for when used in a trailblazer cell #670

Open
sebabouche opened this issue Dec 7, 2015 · 26 comments

Comments

@sebabouche
Copy link

When transposing the trailblazer demo app Gemgem (which is using haml) to slim, we are facing a problem with slim escaping fields_for or simple_fields_for.
capture d ecran 2015-12-07 a 21 01 16
This seems to come from ActionView escaping everything and everywhere.

Haml has solved it with gem "haml", github: "haml/haml", ref: "7c7c169" which should be merged soon.

That doesn't make %haml cooler than slim ;) Would be great if we could solve this for slim too in a future version!

The slim trailblazer demo app (reproducing the problem) can be seen here: https://github.com/sebabouche/traildemo/tree/1bd1c96e760c8811ef14ad50d784f3351774e667

@sebabouche sebabouche changed the title Problem with slim escaping fields_for / simple_fields_for when used in a trailblazer cell Problem with escaped fields_for / simple_fields_for when used in a trailblazer cell Dec 7, 2015
@smathy
Copy link

smathy commented Dec 9, 2015

Just for reference, the haml issue was resolved by haml/haml@297c0d9

@minad
Copy link
Member

minad commented Jan 4, 2016

This is impossible to debug for me without looking into trailblazer which I don't know. Could you help me with a minimal runnable example showing the undesired behaviour.

@sebabouche
Copy link
Author

Hey, I created a mini project here : https://github.com/sebabouche/trb-slim-nested-form-issue.

I could explain you how things are working with TRB by chat?

@sebabouche
Copy link
Author

And I pushed it on heroku here : https://trb-slim.herokuapp.com

@minad
Copy link
Member

minad commented Jan 4, 2016

Ok thx, I will take a look

@minad
Copy link
Member

minad commented Jan 4, 2016

@sebabouche This is not at all a minimal demo. I have limited time to debug such things which are not directly related to slim but more to framework integration. Also the haml fix is totally unrelated since it removes some haml monkey patch which slim doesn't have.

Did you already ask @apotonick about it? Is there something weird going on concerning the capturing?

@minad
Copy link
Member

minad commented Jan 4, 2016

Does it work correctly if you use == instead of = in the form? (This is not a fix, just to see if it would work)

@apotonick
Copy link

Thanks @minad. The problem is that slim HTML-escapes a string even though it's turned off in cells-slim: https://github.com/trailblazer/cells-slim/blob/cce8e43924ec1d2c32f51dc426e20061e5723102/lib/cell/slim.rb#L13

@minad
Copy link
Member

minad commented Jan 4, 2016

@apotonick Ok, however I don't think this is a good idea. Is it not possible to just mark the strings html_safe and let slim do the work?

@minad
Copy link
Member

minad commented Jan 4, 2016

@apotonick You should probably also use the Slim::RailsTemplate

@sebabouche
Copy link
Author

@minad , I'll shrink the mini project to refom + cell + slim. It seems that it's not related to trailblazer.
And == doesn't solve the problem.

@apotonick
Copy link

No way, Cells is completely decoupled from Rails, that's why it's so fast and popular. 😬

We don't use html_safe for the same reason - in Cells, it works differently: We don't need to escape every possible string a million times, since all data in the view is injected via reader methods on the cell instance. That in turn can be escaped by the cell - the template engine shouldn't do any escaping.

By having that strong interface, we are about 10x faster than Rails (and less code).

@sebabouche You don't even need Reform, just the form_tag will do.

@minad
Copy link
Member

minad commented Jan 4, 2016

Ok I understand. Can you take a look at the code that slim generates since you say that it escapes despite being told not to do so? Just access the @src variable of the compiled template.

See here:
https://github.com/judofyr/temple/blob/master/lib/temple/templates/tilt.rb#L30

@sebabouche
Copy link
Author

    @output_buffer = []; _temple_html_pretty1 = /<code|<pre|<textarea/; @output_buffer << ("<h3 class=\"page-header\">\n  What do you want to talk about ?\n</h3>".freeze); 
    ; 
    ; _slim_controls1 = simple_form_for contract, html: {class: css_class} do |f|; _slim_controls2 = ''; 
    ; 
    ; _slim_controls2 << ((::Temple::Utils.indent_dynamic((f.error_notification), true, "\n", _temple_html_pretty1)).to_s); 
    ; 
    ; _slim_controls2 << ((::Temple::Utils.indent_dynamic((f.input :name, placeholder: "Name", label: false, readonly: contract.readonly?(:name)), false, "\n", _temple_html_pretty1)).to_s); 
    ; _slim_controls2 << ((::Temple::Utils.indent_dynamic((f.input :description, placeholder: "Description", label: false), false, "\n", _temple_html_pretty1)).to_s); 
    ; _slim_controls2 << ((::Temple::Utils.indent_dynamic((f.input :file, as: :file), false, "\n", _temple_html_pretty1)).to_s); 
    ; _slim_controls2 << ("<br />\n<div class=\"panel panel-default\">\n  <div class=\"panel-heading\">\n    <h3>\n      Do you know any authors?\n    </h3>\n  </div>\n  <div class=\"panel-body\">".freeze); 
    ; 
    ; 
    ; 
    ; 
    ; 
    ; if signed_in?; 
    ; _slim_controls2 << ((::Temple::Utils.indent_dynamic((f.input :is_author, as: :boolean, label: "I'm the author"), true, "\n    ", _temple_html_pretty1)).to_s); 
    ; 
    ; end; _slim_controls3 = f.simple_fields_for :users do |ff|; _slim_controls4 = ''; 
    ; if @operation.instance_of? Thing::Create; 
    ; _slim_controls4 << ((::Temple::Utils.indent_dynamic((ff.input :email), false, "\n    ", _temple_html_pretty1)).to_s); 
    ; else; 
    ; _slim_controls4 << ((::Temple::Utils.indent_dynamic((ff.input :email, readonly: ff.object.readonly?), false, "\n    ", _temple_html_pretty1)).to_s); 
    ; if ff.object.removeable?; 
    ; _slim_controls4 << ((::Temple::Utils.indent_dynamic((ff.input :remove, as: :boolean, input_html: {checked: false}), false, "\n    ", _temple_html_pretty1)).to_s); 
    ; 
    ; end; end; _slim_controls4; end; _slim_controls2 << ((::Temple::Utils.indent_dynamic((_slim_controls3), false, "\n    ", _temple_html_pretty1)).to_s); _slim_controls2 << ("\n  </div>\n</div>".freeze); _slim_controls2 << ((::Temple::Utils.indent_dynamic((f.submit), true, "\n", _temple_html_pretty1)).to_s); 
    ; _slim_controls2; end; @output_buffer << (::Temple::Utils.indent_dynamic((_slim_controls1), false, "\n", _temple_html_pretty1)); @output_buffer = @output_buffer.join("".freeze)

@RagunovichVlad
Copy link

@minad @apotonick is this bug already fixed?

@rspiridonov
Copy link

I have the same problem with fields_for, cells and slim so the bug still exists.

@kreintjes
Copy link

Same problem over here

@visoft
Copy link

visoft commented Oct 26, 2016

Same problem for me using Slim + fields_for + Spree CMS 3.1.1. Tried making a partial in Slim and it escapes the content. Is there a workaround at all? I hate to have to use ERB. Tried == form.fields_for, but no dice. I'm not using cells at all, so I'm wondering if I should create another issue. It's just a simple form_for with fields_for

Example:

= render partial: 'line_item', collection: order_form.object.line_items, locals: { order_form: order_form }

And in my partial, I had order_form.fields_for :line_items, line_item do |item_form|, everything after that got escaped. My whole project is written in Slim except for this partial that uses fields_for. I ended up just coding it in ERB (yuck), which is really unacceptable.

@tjjjwxzq - I appreciate you showing an example, but I wasn't able to get this to work properly in my project, not sure what I was doing wrong.

@tjjjwxzq
Copy link

tjjjwxzq commented Dec 30, 2016

This is still an issue. In Rails (though I suppose this could be generalized), I figured a workaround which involves putting all the stuff under fields_for in a separate view, then rendering it unescaped:

= form_for @object do |f| 
...
  = CGI::unescape_html(self.(:fields_for_something, f).to_str)

Note: I have to call to_str here on the render output for reasons I don't entirely understand. Basically the output rendered by the cell is an ActiveSupport::SafeBuffer (again, this is a Rails project), and calling CGI::unescape_html on that seems to lead to some nil error, so I convert it to a proper String object first.

@havran
Copy link

havran commented Jan 16, 2017

Same problem here.

Using rails 5.0.1
Using cells 4.1.5
Using cells-slim 0.0.5
Using cells-rails 0.0.6
Using slim 3.0.7

@tarvos21
Copy link

tarvos21 commented Jan 19, 2017

SyntaxError: (__TEMPLATE__):1: syntax error, unexpected ';'
_erbout = []; _temple_html_pretty1 = /<code|<pre|<textarea/; --;^
(__TEMPLATE__):3: syntax error, unexpected ';'
; --;^

I got this error message when using slim, what's the problem? Anyone help? Thanks

@apotonick
Copy link

That looks like you're using ERB? @tarvos21

@tarvos21
Copy link

@apotonick, oh yes, it's in nanoc, a site generator, with slim, but I don't know what's the problem here.

@mizalewski
Copy link

mizalewski commented Jul 8, 2017

I have the same problem. I found workaround, which works for me.
I used inline block and raw method to render fields.

= raw(f.fields_for(:files) { |files_f| files_f.file_field :file })

@gregnavis
Copy link

In my case fields_for didn't work but this did:

== f.simple_fields_for(:children) { |form| cell(AnotherCell, form.object, f: form).to_s.html_safe }.to_s

@ramontayag
Copy link

Thanks opted that too, though I didn't need to to_s:

== f.simple_form_for(@model) {|form| cell(FormContents, form).() }

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