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-next-page: Image URLs aren't rewritten from canonical to cache #15700

Closed
peterjosling opened this issue May 31, 2018 · 6 comments
Closed

Comments

@peterjosling
Copy link
Contributor

What's the issue?

amp-next-page uses canonical URLs for images in the recommendation box when being served from the cache. It should be transforming them to cache URLs a la amp-img

@peterjosling
Copy link
Contributor Author

Can you please reopen this? #15699 is a separate issue (the document URLs weren't getting rewritten properly), this is just to note that the image URLs are always going to point to the canonical source, and need rewriting separately.

@aghassemi aghassemi reopened this May 31, 2018
@aghassemi
Copy link
Contributor

@peterjosling Got it.

This would require AMP Cache to understand the JSON data and do replacement on it during ingestion. /cc @Gregable

and if we ever allow publishers to use an endpoint for JSON data instead of inlining it, we won't be able to do it anymore for remote src cases.

@aghassemi
Copy link
Contributor

One possible approach to this with does not require cache changes is forcing authors to put the images in the page using amp-img id=foo1 layout=nodisplay and then just point to the id in their JSON instead of Url.

This approach would allow use of srcset as well which currently is not supported.

@ampprojectbot
Copy link
Member

This issue seems to be in Pending Triage for awhile. @peterjosling Please triage this to an appropriate milestone.

1 similar comment
@ampprojectbot
Copy link
Member

This issue seems to be in Pending Triage for awhile. @peterjosling Please triage this to an appropriate milestone.

@stale
Copy link

stale bot commented Apr 23, 2021

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 stale bot added the Stale Inactive for one year or more label Apr 23, 2021
@stale stale bot closed this as completed Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants