Prerender amp-video poster image. #25456
Conversation
|
I don't believe this is safe as-is, allowing prerender will cause the element to be built, which will trigger Perhaps we could check if the current document state is prerender in |
|
Great catch, thanks! |
| const {element} = this; | ||
| const sources = toArray(childElementsByTag(element, 'source')); | ||
| sources.push(element); | ||
| for (let i = 0; i < sources.length; i++) { | ||
| if (this.isCachedByCDN_(sources[i])) { | ||
| return true; | ||
| return sources[i]; |
There was a problem hiding this comment.
This returns an element, not a string.
There was a problem hiding this comment.
Done, and added unit tests for preconnectCallback.
|
Don't we also need to check whether the |
| const {element} = this; | ||
| const sources = toArray(childElementsByTag(element, 'source')); | ||
| sources.push(element); | ||
| for (let i = 0; i < sources.length; i++) { | ||
| if (this.isCachedByCDN_(sources[i])) { | ||
| return true; | ||
| return sources[i]; |
There was a problem hiding this comment.
Done, and added unit tests for preconnectCallback.
* Prerender amp-video poster image. * Preconnect to the first cached URL during prerendering. * More unit tests for preconnect.
amp-videoused to load the poster image in prerender (#1657 + #1718).When introducing prerendering for cached videos (#12193 + #17493), we broke the case for prerendering the poster when the video is not cached.
If the video has cached sources OR a poster image, it should allow prerendering.
It is safe to prerender a video with a poster image but no cached source, as the
amp-video.layoutCallbackwill only propagate the cached source until the viewer is visible (source).Fixes #25446