search

ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

[amp-story] When not coming from cache, you should be able to prerender assets without playing the story. #37785

Closed edoudou closed 1 year ago

edoudou commented 2 years ago

Description

Right now, the only way to display an asset is to set the visibility state to visible. However, this play the story. You cannot render the story if you never played it before.

As discussed with @newmuis during the working group, this is to prevent security issues. This prevent tiers services from getting information on the user.

However, this prevent certain worklflow, where a story could be visible but should not play, as it is not the focus of attention.

Developers should have a way to display assets without playing the story, either using an additional visibility, or with an additional state.

Reproduction Steps

On this sandbox, you can try to change the story visibilityState : https://z9ce0.csb.app/

Relevant Logs

No response

Browser(s) Affected

No response

OS(s) Affected

No response

Device(s) Affected

No response

AMP Version Affected

No response

newmuis commented 2 years ago

Adding in a few folks in hopes that one of them has the right context: @jridgewell @samouri @alanorozco @rsimha

This is effectively a feature request for a modality where PRERENDER does not preserve privacy. Is there a precedent for this?

samouri commented 2 years ago

where a story could be visible but should not play, as it is not the focus of attention

A hacky workaround could be to make it visible and then quickly pause. You can't currently skip straight to paused, because then resources won't have their build & layout callbacks invoked.

Is there a precedent for this?

Not that I know of. The new preview state feels a bit similar in purpose. For off-cache (or even just non-SERP) use cases I don't think we have the same privacy-preserving concerns -- so this seems like a reasonable request to me.

gmajoulet commented 2 years ago

A hacky workaround could be to make it visible and then quickly pause.

+1

I believe you can do this pretty easily from the open source Story Player :))

jridgewell commented 2 years ago

We cannot make changes to the privacy features of PRERENDER state. We could make it so that PAUSED state allows buildCallback.

edoudou commented 2 years ago

A hacky workaround could be to make it visible and then quickly pause.

I did try, and I guess due to some race condition, I had the following bug.

In general, I prefer to prevent using "hacky workarounds".

We cannot make changes to the privacy features of PRERENDER state. We could make it so that PAUSED state allows buildCallback.

Yeah I figured out changing the PRERENDER state behaviour might not be a good idea. Being able to use the PAUSED state skipping the VISIBLE state might be a good option.

edoudou commented 2 years ago

Not that I know of. The https://github.com/ampproject/amphtml/issues/37129 state feels a bit similar in purpose. For off-cache (or even just non-SERP) use cases I don't think we have the same privacy-preserving concerns -- so this seems like a reasonable request to me. I just added the PREVIEW state to the code sandbox to test. It seems to be working perfectly for my use case, I just couldn't find it in the documentation / by browsing the code.

It's is just a documentation issue then.

newmuis commented 2 years ago

I think VISIBLE and PREVIEW both suffer from the fact that there is no way to indicate to the story that it should be paused, without encountering delays and/or race conditions.

Not that I know of. The new preview state feels a bit similar in purpose. For off-cache (or even just non-SERP) use cases I don't think we have the same privacy-preserving concerns -- so this seems like a reasonable request to me.

For the record, the new PREVIEW state will also preserve privacy in much the same way as PRERENDER.

edoudou commented 2 years ago

For the record, the new PREVIEW state will also preserve privacy in much the same way as PRERENDER.

Oh, so I guess PREVIEW is not a viable solution for my case either ^^'

samouri commented 2 years ago

@edoudou + @newmuis: The privacy preserving characteristics of PREVIEW and PRERENDER modes depend on an AMP Optimizer rewriting subresource URLs to point to the cache. For example, redirecting all <amp-img> tag src attributes to point to a cache address instead of the original origin address. This concept doesn't apply off cache.

PRERENDER might work for your use case as long as all the components you care about have prerenderAllowed set to true.

gmajoulet commented 2 years ago

Thank you @samouri for bringing the conversation back on track! visibilityState=prerender will load all cached resources. You're only seeing the assets NOT being loaded because you're not using resources loaded from the AMP cache.

You have several options on how to move forward:

  1. (Preferred) load AMP Cache URLs (cdn.ampproject.org)
  2. Run your documents through the AMP optimizer

For (1) if you use the open source Stories Player, all it takes is adding this parameter that will rewrite your URLs: <amp-story-player amp-cache="cdn.ampproject.org" (documentation). We're constantly improving performance for AMP Cache URLs, as we have more control over the scripts/css/assets.

If you do that, using prerender will load your first page assets.

edoudou commented 2 years ago

I see but this mean that I won't be able to really prerender the stories if my assets are not cached by amp ?

newmuis commented 2 years ago

If the documents are valid AMP, they will be able to be served by the AMP cache. We generally don't support documents which are not valid AMP, as we cannot reason about their contents. The document you shared appears to be valid AMP, so it should be able to be served from the AMP cache (here it is being served from the cache).

edoudou commented 2 years ago

We try to always keep our stories compliant, but we have better control over the quality of the assets when not coming from amp cache, so we prefer to display the non cached story in our player.

gmajoulet commented 2 years ago

Just want to make sure you're aware of how much performance you're missing out on by not using the AMP Cache URLs.

Your story on the AMP Cache: https://www.webpagetest.org/result/220310_BiDc0X_C4C/?view=webvitals Not on the AMP Cache: https://www.webpagetest.org/result/220310_AiDcP5_DAH/?view=webvitals

You load 2.5x more bytes, and fail both LCP and CLS. Higher quality assets doesn't necessarily mean better experience, you're losing a ton of traffic to load abandonments and much larger bounce rate.

stale[bot] commented 1 year ago

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.