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 Validator considers a reference to template script[type=text/plain] an error #26676

Closed
grapheon opened this issue Feb 8, 2020 · 5 comments · Fixed by #26747
Closed

AMP Validator considers a reference to template script[type=text/plain] an error #26676

grapheon opened this issue Feb 8, 2020 · 5 comments · Fixed by #26747

Comments

@grapheon
Copy link

grapheon commented Feb 8, 2020

What's the issue?

When referenced from <amp-list ... template="my-template"></amp-list> to <script id="my-template" type="text/plain" template="amp-mustache"> ... </template> AMP Validator showing error "Attribute 'template' in tag 'amp-list' contains a value that does not match any other tags on the page".

However, everything will work, but the page will be deleted from Google AMP Cache due to this error.

How do we reproduce the issue?

  1. Add code to AMP page:
<amp-state id="items"><script type="application/json">[]</script></amp-state>
<button on="tap:AMP.setState({ items: items.splice(items.length, 0, items.length) })">Add item</button>
<template id="items-template" type="amp-mustache"><code>{{#.}}, {{/.}}{{.}}</code></template>
<script id="items-scripted" type="text/plain" template="amp-mustache"><code>{{#.}}, {{/.}}{{.}}</code></script>
<amp-list [src]="items" items="." layout="fixed-height" height="32" template="items-template"></amp-list>
<amp-list [src]="items" items="." layout="fixed-height" height="32" template="items-scripted"></amp-list>
  1. View the AMP page.
  2. See AMP Validator errors.

AMP Validator will point to a line with amp-list referring to the template script[type=text/plain] by id items-scripted. However, everything will work.

Source and Screenshots

Source test script: amp-mustache.html.zip

amp-mustache

@grapheon grapheon changed the title AMP Validator considers a link to template script[type=text/plain] an error AMP Validator considers a reference to template script[type=text/plain] an error Feb 11, 2020
@banaag
Copy link
Contributor

banaag commented Feb 11, 2020

cc @ampproject/wg-caching

@twifkak
Copy link
Member

twifkak commented Feb 11, 2020

Sorry for the delay. Here's a minimal test case.

This requirement was added in #24267 (from http://cl/264646527); it seems it was too strict.

The solution is to add attrs { name: "id" add_value_to_set: TEMPLATE_IDS } inside the script tagspec at

# Templates using script[type=text/plain].

But first we need to make sure that all the dependents know how to find templates in script elements.

@twifkak
Copy link
Member

twifkak commented Feb 11, 2020

@twifkak
Copy link
Member

twifkak commented Feb 11, 2020

OK, fix is in master. It may take ~1 week to propagate.

@grapheon
Copy link
Author

@twifkak thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants