-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Please consider allowing configurable escaping #1175
Comments
Have you tried monkey patching Handlebars.escapeExpression? |
@jbboehr That's an idea I hadn't tried. Thanks! I tend to stick to the public API to avoid maintenance issues, but if |
This or creating a handlebars environment with that specific override are the officially supported approach. Something like:
Should work and avoid any potential global concerns (you might have to adjust that a bit, I haven't tested that directly) |
Great, thanks. Is it documented somewhere that I missed? |
I'm not certain, but above would be some of it :) As with any monkey patch I would recommend writing a test to verify that On Fri, Feb 5, 2016 at 9:57 AM Kevin Locke notifications@github.com wrote:
|
Can you tell me in which versions it has worked. I wasn't aware that this has been possible at all. |
@nknapp I just tried version v4.0.5 (which was released before February 2016) and it worked. I personally have a use case for this feature (templates that generate Markdown), and it would definitely be nice to have. |
@yunyu Please adjust this jsfiddle so that it works: If this is really a regression, I will fix it. But if it is a new feature, I need clarity for myself on how the interface should be. My problem is actually that none of the variants that I proposed in #1523 is really good. And monkey-patching is the worst kind of interface IMHO. As an example, I tried to use a simple "toUpperCase" as escape-function. But the ways Kevin described it about does not work with 4.0.5 and it does not with 3.0.0 |
@nknapp See https://jsfiddle.net/o471djbe/ My mistake, I meant |
I wouldn't recommend doing that as well. Globals shouldn't be changed. I'm closing this in favour of #1523. But in the current situation, I cannot really do a lot of open source work (schools closed due to corona, supervise kids I'm addition to working from home, there's not much energy left right now) |
First, thanks for writing/maintaining Handlebars.js. I've used it on several projects and really like it.
In v4.0.0 and later, due to 83b8e84 (as discussed in #1114 and #1083) ,
=
is now escaped to=
. Although this is probably a good choice on the whole, it is causing issues for me.My immediate issue is that SendGrid doesn't HTML-decode
href
attribute values before replacement in outgoing emails. So templating<a href="{{linkUrl}}">a link</a>
withhttps://example.com/?utm_medium=email
yields<a href="https://example.com/?utm_medium=email">a link</a>
which gets replaced by SendGrid with a tracking URL that redirects to the unescapedhref
value. Clearly their system is broken, but they have no plans to fix it and it is out of my control. It's also likely not the only software which doesn't properly HTML-decodehref
values.I am currently doing my own escaping and returning
SafeString
, but the amount of work to do this consistently is non-trivial. It would be nice to have an easier option.Since I always quote attribute values, the
=
escaping is not necessary and counter-productive for my uses. Would you be willing to support an option to configure which characters are escaped, to accommodate differing needs/uses? If so, I could work up a PR.Thanks,
Kevin
The text was updated successfully, but these errors were encountered: