Significant change should be better explained in the Changelog regarding non ERB view partials

[Rodrigo Rosenfeld Rosas]

Here is the relevant section in the changelog, I think:

https://github.com/rails/rails/blob/v5.0.0.beta4/actionview/CHANGELOG.md

"Change the default template handler from ERB to Raw.

Files without a template handler in their extension will be rendered
using the raw handler instead of ERB.

Rafael Mendonça França"

I always give it a glance to the changelogs before trying to upgrade but
this documented change didn't grab my attention enough to think that it
might impact on my application.

There are a few performance/analytics/monitoring related scripts that
are loaded directly in an inline script in one of my view layouts.

They are loaded like this:

<script type="text/javascript">
<% unless params[:monitoring] || Rails.env.development? -%>
<%= render '/common/pingdomjs.js' %>
<% end -%>
...

After this change I had to move it to:

<%= render('/common/pingdomjs.js').html_safe %>

I think this is not an uncommon pattern and that maybe the need of such
changes to non ERB files rendering should be highlighted in the
Changelog to grab the reader's attention.

This is just a sugestion to improve the changelog before Rails 5 final
is released in order to make the upgrade process smoother.

Best,
Rodrigo.

[Rafael Mendonça França]

That makes sense. Thank you for pointing out. Do you have a suggestion how to improve it? Could you open a pull request?

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

[Rodrigo Rosenfeld Rosas]

Hi Rafael, thanks for your response. My wife went through a surgery on Friday and is under recovery for a few days and I'm alone with the babies during the weekend. I'll try to find some time to create such PR tomorrow.

But, before I do that, I'd like to ask: is this incompatible change really intentional?

I mean, I can see why Raw should be the default over ERB, but shouldn't the resulted string be flagged as html safe by default?

In other words, could you describe a real scenario where applying html escaping over a raw partial would make sense?

I'm just curious. If you decide this change indeed worths the breakage I can try to submit a PR with a better warning complementing the current explanation.

Best,
Rodrigo.

[Rafael Mendonça França]

Hi, no worries. I can think in a better wording open a PR and ping you on it.

Yes, the change is intentional and it is not html safe by default by definition. The whole idea of this change is to avoid people to rendering unsafe responses as safe by mistake.

See https://github.com/rails/rails/issues/2394 and https://github.com/rails/rails/pull/6309 for background.

[Rodrigo Rosenfeld Rosas]

I understand the reasons why it's usually not desired for strings to be html safe by default but I don't see how useful this change is when applied to partials rendering, that's why I asked for an example. I couldn't find any real scenario where it would be desired when looking at the issues you mentioned.

I understand why we don't want every file to be interpreted by ERB by default and why RAW is a better fit, I just fail to see how not being html safe would be useful for partial rendering...

The babysitter should arrive in a few minutes and I'll try to open a PR to change the changelog anyway...

[Rodrigo Rosenfeld Rosas]

Or maybe we should automatically flag .js and .htm(l) extensions as html safe after using the raw handler, as they are common extensions expected to be used as partials...