helpers.f doesn't convert to string first as it used to

[magopian]

Since https://github.com/jbalogh/jingo/commit/b40ccfcba362092a79001aa5c0da77fd904ed692#diff-ef2d1aabcfb813b2ee524b54f1a93742L26 the helpers.f helper doesn't convert its input to a string before doing the string interpolation, like it used to.

This conversion, however, is still done in helpers.fe.

This causes issues when the input is a jinja2.Markup object, which automatically escapes anything that is string interpolated with it.

This happens, for example, if you're using https://github.com/clouserw/tower/, which returns a jinja2.Markup object for localized strings:

{{ _('this is {0}')|f('<b>markup</b>') }}

This is supposed to return This is <b>markup</b>, but instead returns This is <b>markup</b>

[jsocol]

This is working as intended.

You are passing unsafe (e.g. a string, not a Markup) content into the filter, so it's being escaped. The correct way to fix this—and I'd argue that doing this is, itself, incorrect and unsafe—is to mark the input variable as safe:

{{ _('this is {0}')|f(Markup('<b>unsafe</b>')) }}

Really, you should be wrapping it in Markup when it's generated, in the view, not the template, so you can be certain it's safe before it gets marked safe.

[jsocol]

Wait, OK, I see what you mean. Hmm.

[fwenzel]

I see how this is a regression, but frankly I am not sure why this behavior is wrong -- isn't the exact point of the format helper to interpolate potentially unsafe strings into a safe string?

[magopian]

Hey @jsocol, to be honest, I'm not quite sure about this PR either.

I thought the unicode call was removed (and not replaced) by error during the py3 compatibility work, because the call to unicode was replaced by six.text_type a few lines after (https://github.com/jbalogh/jingo/commit/b40ccfcba362092a79001aa5c0da77fd904ed692#diff-ef2d1aabcfb813b2ee524b54f1a93742L35).

As @fwenzel states, it did feel like a regression, however, I'm not it's desirable. Looking back to this issue, I'm feeling the issue is more because of https://github.com/clouserw/tower/commit/1e06110269fcd7ef54c94e9f281bc05a4996c6e2#diff-df82820766b6c988de19ad765e5f0adbL9 instead.

Anyway, if somebody is reading this and needing a way to fix it (if we don't merge this PR), here's a way to make it work (the proper way?):

{{ _('this is {0}')|f('<b>markup</b>'|safe) }}

If you decide that this is the intended behavior, maybe I can create another PR (the opposite ;) removing the call to six.text_type in the helpers.fe helper?

[jsocol]

Continuing on #54.