amphtml icon indicating copy to clipboard operation
amphtml copied to clipboard

Stop cache rewriting of URLs in style attributes of templates

Open dreamofabear opened this issue 6 years ago • 5 comments

Background

Currently, URLs inside the style attribute of <template> descendants are rewritten as AMP cache URLs (on pages served from AMP cache). For example:

<!-- https://foo.com/ -->
<template type="amp-mustache">
  <div style="background-image: url('x.jpg')"></div>
</template>
<!-- https://foo-com.cdn.ampproject.org/v/s/foo.com/ -->
<template>
  <div style="background-image: url('https://foo-com.cdn.ampproject.org/i/foo.com/x.jpg')">
  </div>
</template>

This can cause bugs when the URLs have template variables, e.g. #19439. To avoid these bugs and improve consistency with existing behavior, we're planning to disable cache rewriting of these URLs.

This is a potentially breaking change for pages served from AMP cache.

Am I affected by this change?

If you're using relative URLs inside style attributes for descendants of <template> elements, yes.

Otherwise, no.

I'm affected. How do I fix it?

Change your relative URLs to be absolute. E.g. for the above example:

<template type="amp-mustache">
  <!-- Only use absolute URLs inside templates. -->
  <div style="background-image: url('https://foo.com/x.jpg')"></div>
</template>

When will this change be made?

There's no timeline yet. First, we'll add code to detect these cases at runtime and output a user error. We'll update this issue with sufficient forward notice per the deprecations policy if a formal deprecation is warranted.

dreamofabear avatar Nov 27 '18 21:11 dreamofabear

For the cache piece of this, do we want to disable all URL rewrites inside a <template> tag, or should we try to be smarter? For example, we could rewrite if the URL does not contain {{.

Disabling all is easier to reason about, so is what I recommend.

Gregable avatar Jan 28 '20 17:01 Gregable

@choumx Is this still something we want to implement? Has the runtime change been implemented?

Gregable avatar Jan 29 '21 01:01 Gregable

Sorry, dropped the ball on this. @jridgewell can you follow up?

dreamofabear avatar Feb 04 '21 19:02 dreamofabear

@samouri Do we rewrite template CSS urls at runtime? If we don't, disabling the Cache rewriting would cause all CSS URLs to break.

jridgewell avatar Feb 04 '21 21:02 jridgewell

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 30 '22 18:07 stale[bot]