ampproject / amphtml

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

[amp-story-player] Story not immediately loading in player, needs user interaction #35959

Closed swissspidy closed 1 year ago

swissspidy commented 3 years ago

Description

I originally mentioned this at https://github.com/ampproject/amphtml/issues/34271#issuecomment-907008438, but I think it's a separate issue altogether.

For the WordPress story editor we've been receiving some reports where stories being opened in a lightbox (using amp-story-player) are not immediately visible.

There's some interesting behavior where the amp-story-player seems to load, but the story is not visible until some user interaction (i.e. press esc or click on the element).

Only after that interaction does the embedded story in the iframe have the i-amphtml-story-standalone classname.

From what I can see, that class name is added in the buildCallback of the amp-story component, so for some reason that is not being triggered initially:

https://github.com/ampproject/amphtml/blob/e1edbde829a6ffeaba986e5cc873359bd7322049/extensions/amp-story/1.0/amp-story.js#L518-L529

/cc @ampproject/wg-stories @Enriqe

Reproduction Steps

  1. Visit https://cai2r.net/
  2. Scroll down to where a story is embedded (there's a poster image with the headline on top of it)
  3. Click on the poster image to open the amp-story-player in the iframe
  4. See black screen
  5. Click on center of the screen or press a key or something
  6. See how the story appears

Here's a screenshot showing the missing i-amphtml-story-standalone classname at the beginning:

Screenshot 2021-09-06 at 11 49 56

Here's a screencast showing the issue:

https://user-images.githubusercontent.com/841956/132198130-4bcb964e-0274-4f0c-9e08-5e2ed178b055.mov

Relevant Logs

Uncaught (in promise) Error: No messaging channel: initMessagingChannel timeout
    at ia (index.js:43)
    at tb.createError (log.js:295)
    at timer-impl.js:129
    at timer-impl.js:70

Browser(s) Affected

No response

OS(s) Affected

No response

Device(s) Affected

No response

AMP Version Affected

2108192119000

gmajoulet commented 3 years ago

That sounds pretty bad, we've seen a ton of errors in our logs starting from 09/03, originating from a Player that would show Stories in their prerender mode. That might probably be it.

cc @ampproject/wg-stories

pawelslabiak commented 3 years ago

@swissspidy, thank you for raising this issue. Because of this bug, we have taken down the web story carousel so the web page linked above no longer contains a live example of the bug.

processprocess commented 3 years ago

Because of this bug, we have taken down the web story carousel so the web page linked above no longer contains a live example of the bug.

Hi @pawelslabiak, I'm debugging this and am unable to reproduce the bug locally. Is it possible to host a page on that website with the carousel and post a link?

pawelslabiak commented 3 years ago

Hi @processprocess, thank you for looking into this. We've created a page with a stories block at cai2r.net/7079-2/ However, the observed behavior is now slightly different (perhaps as a result of updating Wordpress to version 5.8.1 and the Web Stories plugin to version 1.11.0).

On a desktop chromium-based browser, we observed the following:

On a mobile chromium-based browser, we observed the following:

On a desktop safari browser:

processprocess commented 3 years ago

Thanks for positing the demo page. Happy to hear that it's working after updating but it sounds like it's slow and at times still requires interaction.

I would like to try testing something else: Is it possible to add some empty blocks of content above the stories block so we scroll to it? We have some code related to scroll position so I want to see if that's part of the issue.

pawelslabiak commented 3 years ago

Absolutely—we appreciate your investigating this issue. The page now has a number of spacers at the top to make scrolling necessary to get to the stories block.

processprocess commented 3 years ago

It looks like both stories in the carousel are failing validation:

https://validator.ampproject.org/#url=https%3A%2F%2Fcai2r.net%2Fweb-stories%2Fai-can-reduce-overdiagnosis-in-ultrasound-screening-for-breast-cancer

https://validator.ampproject.org/#url=https%3A%2F%2Fcai2r.net%2Fweb-stories%2Fscientists-propose-new-standard-for-imaging-myelin

I'm not sure if this would effect the bug or not but it would be good to fix this and re-load the carousel to rule that out.

gmajoulet commented 3 years ago

Updating your Story with a valid one (cf @processprocess) seems to work for us.

If you copy/paste this snippet in the console, and click on the first card, it should load correctly.

$('amp-story-player').add([{'href':'https://www.washingtonpost.com/photography/interactive/2021/profound-memorial-shanksville/', title:'foo'}]);
$('.web-stories-list__story').setAttribute('data-story-url', 'https://www.washingtonpost.com/photography/interactive/2021/profound-memorial-shanksville/');
pawelslabiak commented 3 years ago

@processprocess, the black screen issue precedes the AMP validation problem, which is new and seems to arise from another plugin (Yoast SEO) generating an extraneous canonical tag for Web Stories. For the time being, we are filtering those out using the workaround suggested by Mike Zielonka in the relevant Github issue thread.

After implementing the filter, stories pass AMP validation but the open behavior remains unchanged.

gmajoulet commented 3 years ago

As per https://github.com/ampproject/amphtml/issues/35959#issuecomment-917163537, the issue seems to be content based. We're obviously here to help, but could you please help us and send us a codepen or anything we can edit, so we can debug faster and figure out what's wrong?

pawelslabiak commented 3 years ago

@gmajoulet would love to help further but not sure I understand what exactly you're requesting. Feel free to email me if having an aside offline would be easier.

swissspidy commented 2 years ago

@gmajoulet I put together a CodePen for you here: https://codepen.io/swissspidy/pen/eYRGoBN

Same issue happens there.

swissspidy commented 2 years ago

@processprocess @gmajoulet friendly ping :wave: have you had a chance to look at the CodePen?

processprocess commented 2 years ago

Thank you for following up @swissspidy and posting the codepen. I'm able to reproduce the bug with that and will look into this.

gmajoulet commented 2 years ago

Is this still a problem?

swissspidy commented 2 years ago

To my knowledge, yes.

The issue can still be reproduced with the above CodePen.

See how only the player's arrows are visible when opening the lightbox, but not the actual player contents:

Screenshot 2021-10-25 at 22 57 59

After an additional click, the player contents appears.

pawelslabiak commented 2 years ago

Yes @gmajoulet , as @swissspidy notes above, this continues to be an issue.

mxbclang commented 2 years ago

@gmajoulet Checking in here as we continue to get reports of this issue on the Web Stories forum.

DanielCryptoK commented 2 years ago

I am still getting this error, please can someone let me know when it will be fixed or if there is anything I can do to fix it my end as been many months this problem has been effecting my website for now.

pawelslabiak commented 2 years ago

This buggy behavior also continues on our website.

DanielCryptoK commented 2 years ago

Is this problem every going to be fixed as reported way back in April and no one seems to be working on fixing it.

DanielCryptoK commented 2 years ago

It is ONE YEAR since this error was reported. Can someone please let me know if it is going to be fixed?

processprocess commented 2 years ago

Thank you for following up. This demo of a player in a lightbox appears to be working as intended.

Since it's been a while, can you post an example so we can repro?

DanielCryptoK commented 2 years ago

Hi

Yes no problem see video of it not working. It has never worked for me.

https://www.awesomescreenshot.com/video/10928529?key=1726e40417be7d0be880a6863c464e33

You can test for yourself by visiting https://bitcoin-casino-no-deposit-bonus.com/

DanielCryptoK commented 2 years ago

Also please can you take a look at this ticket I have raised that no one in the Google Community seems to be able to answer.

https://support.google.com/webmasters/thread/177831693?hl=en

DanielCryptoK commented 2 years ago

Sorry I may as well list all the problems that I have been trying to fix for months but no one seems to fix even though I have reported here. I believe this error is linked to web stories????

index.js:43 Uncaught (in promise) Error: No messaging channel: initMessagingChannel timeout at o (index.js:43:13) at Et.createError (log.js:295:45) at timer-impl.js:130:23 at timer-impl.js:71:9

DanielCryptoK commented 2 years ago

Sorry I also forgot to mention the below error that I reported months ago. For some strange reason the ticket has been closed but the problem is still showing. This is the github post it was first reported on and is now closed https://github.com/GoogleForCreators/web-stories-wp/issues/11306

amp-story.js:1284 Uncaught TypeError: Cannot read properties of null (reading 'next') at jn.Er (amp-story.js:1284:12) at jn.nz (amp-story.js:1366:12) at amp-story.js:700:12 at page-advancement.js:229:7 at Array.forEach () at Vo.onTapNavigation (page-advancement.js:228:34) at Vo.Zx (page-advancement.js:736:10)

DanielCryptoK commented 2 years ago

This is the ticket I reported the errors on https://github.com/GoogleForCreators/web-stories-wp/issues/12047

newmuis commented 2 years ago

Regarding the console errors, AMP pages do sometimes log things to the console, so that is, to some degree, working as intended. These error reports help the AMP community debug issues as they arise, and provide insight as to what users might be experiencing on the page.

Regarding the Web Stories Video Index Problem thread, I believe you have already reported this to the correct channels. This repository is for the open source AMP project; we have no insight into what might be happening with any particular platform's indexing pipelines.


As for the issue at hand with the story not loading in the player, yes it looks like the messaging channel is not established. The story is correctly in the PRERENDER before the user clicks on the entry point, and when the user clicks the entrypoint, a message is correctly sent from the viewer to transition the story into VISIBLE mode. However, because the messaging channel is not properly established, the story never receives this message, and so it remains stuck in the PRERENDER mode when it enters the user's viewport. It is rendered correctly when interacted with due to the workaround introduced in #28227.

So, the open question is: why does initializing the viewer messaging handshake (sometimes) fail?

/cc @gmajoulet @coreymasanto who may have context on that part

coreymasanto commented 2 years ago

So, the open question is: why does initializing the viewer messaging handshake (sometimes) fail?

I believe that the handshake request is sent from the story to the amp-story-player here and the handshake response is then supposed to be sent from the amp-story-player to the story here. In my opinion, the most likely possibilities are that the amp-story-player is either 1) never receiving the request and thus incapable of returning a response, or 2) mishandling the request in such a way that the 'channelOpen' handshake response is not being sent to the story. I'll set up breakpoints and try to repro so that we can figure out which scenario is occurring.

gmajoulet commented 1 year ago

Hello, These are very hard to debug as you're using custom CSS to embed our code.

The fact that the handshake request is never sent makes me think that the elements never built. This could very likely be because the elements have no size, we have an optimization that says that if the content is hidden, we don't build it.

You're wrapping our player inside some divs with these classes web-stories-list__lightbox-wrapper ws-lightbox-1 but also web-stories-list__lightbox. They have display: none which completely hides the content and override the CSS we distribute. Please give a size to your lightbox and don't hide it (display: none), I believe that'll fix it.

DanielCryptoK commented 1 year ago

Hi gmajoulet,

Many thanks for looking into this for me.

I did not realize there was any custom css which I have now found and removed the below

.web-stories-embed:not(.web-stories-embed-amp) .wp-block-embed__wrapper { max-width: none; }

.alignnone { margin: 1.5em auto; padding: 0 30px; }

.web-stories-embed .wp-block-embed__wrapper { margin-left: auto; margin-right: auto; }

However the problem is not fixed and still requires two clicks to open a web story.

gmajoulet commented 1 year ago

Can you link to the updated website please? I've seen a few display: none; statements etc, I can help look into what else needs to be removed.

DanielCryptoK commented 1 year ago

The live version is the updated version with the custom web stories code removed. https://bitcoin-casino-no-deposit-bonus.com/

Would you like me to post all of custom code for CSS/JS here, maybe problem there?

Also not one person has been able to answer my ticket as to why certain videos are not been indexed by google. It is greatly damaging my site so if you have any ideas why this is the case it would be much apricated as no one in the google community can answer it. https://support.google.com/webmasters/thread/177831693?hl=en

gmajoulet commented 1 year ago

You have a display: none on .web-stories-list__lightbox that hides the amp-story-player. The player is positioned away from the screen so I don't think you need this display none.

You can test your code by running this in the console, and making sure that width and height are NOT 0. $('amp-story-player').getBoundingClientRect()

DanielCryptoK commented 1 year ago

Wow that is so over my head as I no idea how to do that. Are you able to tell me exactly where I would find the code that needs to be changed in wordpress as I can not find any of these web stories lightbox settings???

swissspidy commented 1 year ago

@gmajoulet For context, that CSS is coming from the Web Stories WordPress plugin. Are you saying the player can‘t properly load when it‘s hidden like this? Should calling the player‘s load() method fix this?

swissspidy commented 1 year ago

@gmajoulet could you perhaps point me to that optimization you mentioned? I fail to reproduce this locally with similar CSS.

@DanielCryptoK Try adding this to your "Additional CSS" section in the WordPress customizer:

.web-stories-list__lightbox { display: block !important; }
DanielCryptoK commented 1 year ago

The code worked but there is a hidden menu at the top. See image of what I mean here https://www.webpagescreenshot.info/#v2=r78urBJUv

swissspidy commented 1 year ago

This is because of something on your website removing the CSS from https://cdn.ampproject.org/amp-story-player-v0.css. Either try disabling all other plugins to find the culprit or add this to "Additional CSS" as a workaround:

amp-story-player.i-amphtml-story-player-loaded a { display:none !important; }
DanielCryptoK commented 1 year ago

Could it be the below error that is caused by my super progressive apps plugin which the developer knows about and has been trying to fix for the last two months.

superpwa-sw.js:9 Uncaught (in promise) TypeError: Failed to execute 'put' on 'Cache': Partial response (status code 206) is unsupported at superpwa-sw.js:9:178

swissspidy commented 1 year ago

That looks unrelated

newmuis commented 1 year ago

I'll also note that in this specific case, it looks like the css is being served from https://bitcoin-casino-no-deposit-bonus.com/wp-content/cache/min/1/amp-story-player-v0.js?ver=1663611239 which looks like it (a) is caching the css, and (b) is changing/minifying the css, both of which are potentially problematic.

I can't actually determine whether that is affecting things in this scenario, but also the repro case above at https://cai2r.net/7079-2/ seems more minimal, as it is loading the scripts/styles as expected.

Ideally we would find a way to construct a truly minimal reproducible example for this issue to be actionable.

DanielCryptoK commented 1 year ago

Would excluding web stories from the CSS option in WP rocket solve this issue? See image https://www.webpagescreenshot.info/#v2=pz4opMOAB

swissspidy commented 1 year ago

@DanielCryptoK Maybe, but you would need to exclude https://cdn.ampproject.org/ there. But if that does not help, please ask about it in the Web Stories WordPress plugin forums, in order to not further derail the discussion here.

DanielCryptoK commented 1 year ago

Ok, just so you know excluding the web stories plugin from the WP rocket CSS settings fixed the Super progressive web app error which was a nice surprise.

gmajoulet commented 1 year ago

Glad we solved this :)) Thanks all!

newmuis commented 1 year ago

It's not clear to me: did this resolve the original issue? Or just the unrelated JavaScript error?

pawelslabiak commented 1 year ago

@newmuis, the original issue appears not have been solved.

@swissspidy @gmajoulet et al., The problem with web stories launching continues. For reference, see the test page we set up earlier at the request of @processprocess: https://cai2r.net/7079-2/ (scroll down for the web stories).

Really appreciate everyone's attention to this issue.

gmajoulet commented 1 year ago

Who owns the web-stories-list__lightbox CSS? The issue should be filed against this team so they fix their CSS, removing the display: none; and ensuring the amp-story-player has a size.