Amphtml: "External User Consent Flow" isn't working

[agarmave]

Please only file reports about bugs in AMP here.

• If you have questions about how to use AMP or other general questions about AMP please ask them on Stack Overflow under the AMP HTML tag instead of filing an issue here: http://stackoverflow.com/questions/tagged/amp-html
• If you have questions/issues related to Google Search please ask them in Google's AMP forum instead of filing an issue here: https://goo.gl/utQ1KZ

If you have a bug for AMP please fill in the following template. Delete everything except the headers (including this text).

What's the issue?

"External User Consent Flow" isn't working

How do we reproduce the issue?

It can be reproduced in the AMP By Example page: https://ampbyexample.com/playground/#url=https://ampbyexample.com/user_consent/external_user_consent_flow/source/

Or in our page:
https://web.movistar.es/particulares/promociones/prueba-consentimientos

In both cases the AMP-Iframe is not loaded and a black veil the only thing left.

No error is shown in console.

What browsers are affected?

All browsers, all devices

Which AMP version is affected?

Version 1901241837330
This feature worked until last week

THANK YOU VERY MUCH! :)

[mpatnode]

Another test case for you: https://www.google.com/amp/s/www.popsugar.com/celebrity/Victoria-Beckham-Anniversary-Quotes-About-David-2019-45723753/amp

[zhouyx]

@torch2424 Can we please take a look. Thank you

[torch2424]

Hello,

@agarmave @mpatnode @patrick-mcdougle

So I can confirm that the issue on the examples provided. Sorry about that. 😞

But, I went ahead and tested this on the latest master:

Thus I don't think that the call to toggle could be the issue? 🤔

And looking at the example with source provided, it seems that the shade applied is not the one I developed in the PR mentioned by the mask overlay:

As the classes would be much different.

Thus, I think this is a larger issue with the amp-iframe just not loading?

What do you think @zhouyx ? 😄

[patrick-mcdougle]

I think the iframe doesn't load if if it's not in the viewport. If it's hidden, I don't think it's "in the viewport". That was my thought process anyway. I don't have a lot of knowledge about the amp codebase though, so my musings are just hypotheses. Someone should look / debug with more specialized knowledge of how the code works.

My process was as follows: I added a breakpoint in the AMP code where the call to toggle used to be and called it myself. After doing this, I saw the network call to the amp-iframe source in the network pane of the chrome dev tools, so that's where this test / hypothesis came from.

[zhouyx]

Thank you all for filing the issue and debugging.

I've verified moving toggle() around is indeed the cause here. The toggle(iframe, true) was moved after scheduleLayout(iframe). In this case runtime won't layout the iframe because it has display:none. @torch2424 can we please fix the issue and file a request to patch the fix. Thank you

[torch2424]

@zhouyx Thanks for investigating this. Will work on a fix ASAP. 😄

[patrick-mcdougle]

Thanks @zhouyx and @torch2424 for your speedy jump on this.

[torch2424]

Hello,

@agarmave @mpatnode @patrick-mcdougle

So I have a fix out at #20588 . But I want to be fully transparent and let you know how this is gonna work, and all the gotchas we got going on right now:

• The first one: Our team is distributed, and we currently have the NY office in SF, so this is our team outing week. In fact, we just got back from an event, but I stopped by the office to open the PR.

• Usually our release process is done every two weeks, but we'll go ahead and cherry pick this into PROD (or canary, however that works), and then the fix should be out for your users.

• Because of 1, our current release engineer is half out here for fun / half doing releases. So when this gets cherry picked onto PROD depends on more than just my schedule.

Hopefully, @zhouyx Can get this reviewed/approved by tommorow, and I can reach out to the release engineer to get this cherry picked in ASAP.

Thanks for your patience! 😄

[torch2424]

For anyone following this issue, please follow the Cherry Pick issue: #20593 As that will determine when this fix hits PROD. 😄

Thank you for your patience!

[agarmave]

Thank you very much @torch2424 and @zhouyx!!