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

amp-mustache: Auto escaping of quote chars #8395

Closed
dreamofabear opened this issue Mar 24, 2017 · 13 comments
Closed

amp-mustache: Auto escaping of quote chars #8395

dreamofabear opened this issue Mar 24, 2017 · 13 comments

Comments

@dreamofabear
Copy link

No description provided.

@kmh287
Copy link
Contributor

kmh287 commented Apr 4, 2017

Is there a case this doesn't work for? I just tried:

<p [text]="'&quot;' + sentence + '&quot;'"></p>
<button on="tap:AMP.setState({sentence: 'the &quot;button&quot; was pressed'})"></button>

Where pressing the button will change the p tag's text to: "the "button" was pressed"

EDIT: Just realized this issue is for escaping the single quote / apostrophe.

EDIT 2:

<p [text]="'&quot;' + sentence + '&quot;'"></p>
<button on="tap:AMP.setState({sentence: &quot;Don't tread on me!&quot;})"></button>

Pressing the button changes the p tag's text to "Don't tread on me!"

@dreamofabear dreamofabear removed their assignment Apr 5, 2017
@ericlindley-g ericlindley-g added this to Needs Triage in UI May 12, 2017
@ericlindley-g ericlindley-g moved this from Needs Triage to Backlog in UI May 23, 2017
@micyeung
Copy link

Hi - any update on this issue? Quote escaping is particularly important in e-commerce use cases where single / double quotes appear quite frequently.

@dreamofabear
Copy link
Author

Does @kmh287's workaround (above) work for those use cases?

@pbakaus
Copy link
Contributor

pbakaus commented Dec 28, 2017

For single quotes, would &lsquo; (left hand single quote) and &rsquo; (right hand single quote) work instead? They seem to be the ones that carry the right semantic meaning.

@leonahliang90
Copy link
Contributor

leonahliang90 commented Jan 2, 2018

the workaround above is not fitting to all cases. The Edit 1 is to escape double quote and the Edit 2 is to escape single quote. However, at our case, our contents will contain both Single quote & Double quote.
The figures below show if both if both single quote & double quote appear on the line.

code
dingtalk20180102222441

While dealing with this, I have come out a solution, which we can use the Edit 2 approach ( to escape single quote) by changing all the double quote with ″ &Prime to avoid the conflicting of double quote.

However, I am using <amp-mustache> for the templating and I am not too sure why it is not working. Please advise how this can be achieved inside <amp-mustache>, appreciate it.
image

@dreamofabear
Copy link
Author

Not sure I fully understand. In your example above, it looks like the HTML is:

<button class="btn" on="tap:AMP.setState({sentence: &quot;'Boot&apos;s Attributes is 3&quot; and above &quot;})">

Looks like the value of the sentence key is not quoted correctly:

sentence: &quot;'Boot&apos;s Attributes is 3&quot; and above &quot;

What you probably want is this:

sentence: 'Boot&apos;s Attributes is 3&quot; and above'

@leonahliang90
Copy link
Contributor

leonahliang90 commented Jan 3, 2018

Hey William, thank you for the quick feedback, I have tried

What you probably want is this:
sentence: 'Boot's Attributes is 3" and above'

and it is still not working, anyway, I have found a solution for this. Which is :

  1. By changing all double quotes " to Prime ″ before JSON is passed from our backend , to avoid conflicting with <html> double quotes, although there are some outlook different but it is acceptable.
  2. This one is my mistake, I missed out and have reviewed back <amp-mustache> documentation link here , by using triple curly braces{{{attributes}}} and now it is working fine ! case closed.

@leonahliang90
Copy link
Contributor

leonahliang90 commented Jun 19, 2018

triple-curly-braces

Quick update since January, triple curly braces of amp-mustache will not pass the amp-validator, so we moved the Prime ″ to backend instead, rendered the character &Prime before it is passed to amp-list.

@dreamofabear dreamofabear changed the title amp-bind: Add quote-escaping in strings amp-mustache: Auto escaping of quote chars Aug 14, 2018
@dreamofabear
Copy link
Author

Changing this FR to track auto-escaping in amp-mustache since this has come up again. Related: #12969

@dreamofabear
Copy link
Author

Dev-ex would be improved if we can avoid the need for developers to change backend data (e.g. replace ' with ). One idea is to allow a mustache lambda that can perform this specific replacement in sections of template code.

E.g. in the example below, rendering with the bar variable will generate a runtime error if it contains a single or double quote character.

<template type="amp-mustache">
  <button on="tap:AMP.setState(foo: 'Templated {{bar}} needs quote escaping')">
  </button>
</template>

We can support a quotable lambda (name configurable) that does this replacement e.g.

{
  bar: "This single-quote (') causes a runtime error",
  quotable: function() {
    return (text, render) => render(text.replace("'", "′"));
  }
}
<template type="amp-mustache">
  <button on="tap:AMP.setState(foo: '{{#quotable}}{{bar}}{{/quotable}}')">
  </button>
</template>

@swindjam
Copy link

swindjam commented Apr 4, 2019

It's not quite the same issue, but this also happens when using amp-mustache templates in amp-bind.
E.g. data-amp-bind-text=" '{{ offer.name }}' " breaks if the value from the items in an amp-list includes a ' such as Microsoft Xbox One X Console, 1TB, with Wireless Controller, Black and PlayerUnknown's Battlegrounds Game Bundle

We could replace the quote in our api but we don't want to do this to avoid causing edge cases.

Ideally we would just output the data from mustache and escape the strings, but that doesn't appear to be an option at present

@dreamofabear
Copy link
Author

New feature: If you use <script type="text/plain" template="amp-mustache"> instead of a <template type="amp-mustache"> to specify the mustache template string, you can use HTML char codes in the template (e.g. &apos; instead of ') and they will be preserved.

See https://github.com/ampproject/amphtml/blob/master/extensions/amp-mustache/amp-mustache.md#usage for details.

This is probably preferable to the "auto-escaping" proposal above.

@dreamofabear
Copy link
Author

Closing as obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
UI
  
Backlog
Development

No branches or pull requests

7 participants