Amphtml: amp-consent UI does not show when scrolling

Created on 13 Dec 2020  路  15Comments  路  Source: ampproject/amphtml

What's the issue?

When loading an AMP page on the Google CDN (and only on the Google CDN - loading an AMP page directly will not manifest the issue), and amp-consent applies, if you start scrolling as soon as the page loads until amp-consent fully initialises and UI/overlay should show, the overlay shows, but the consent UI doesn't. It is present in the DOM, but it's off the screen due to its CSS styling.
It seems the following CSS isn't being applied to the amp-consent element:

transform: translate3d(0px, calc(100% - 60vh), 0px) !important

Applying this style manually in Chrome inspector makes it appear as normal, and when you load the page without scrolling, and the amp-consent ui element shows as normal, that style seems to be present.

See the contrasting screenshots:

Screenshot 2020-12-13 at 22 10 46

Screenshot 2020-12-13 at 22 11 19

These styles seem to be getting applied here
https://github.com/ampproject/amphtml/blob/master/extensions/amp-consent/0.1/consent-ui.js#L711

The end result is the overlay blocks the screen completely but consent cannot be given, effectively blocking the page for the user.

On iOS Safari devices, in some cases the UI shows and consent can be given, but then the overlay doesn't disappear, and continues blocking the page.

How do we reproduce the issue?

https://www.google.com/amp/s/www.dailymail.co.uk/tvshowbiz/article-9003291/amp/Madonna-gives-fans-rare-look-six-children-family-holiday-photos.html

  1. Load page on a EU geo in incognito mode (or normally, without having consented; clear storage after consenting)
  2. Start scrolling up and down as soon as possible and keep doing so until the amp-consent overlay blocks the screen

This has been reproduced on multiple iPhone, Android phones of various makes, and can be reliably reproduced in Chrome in device emulator mode.

The geo needs to be a EU geo for the amp-consent to appear (via amp-geo "eu" group).

What browsers are affected?

Safari iOS 14 and Chrome, multiple iOS/Android devices tested, Chrome 87 emulation included

Which AMP version is affected?

amp-consent 0.1 - the issue has been detected only recently but I cannot confirm if it wasn't present before

Thanks!

amp-consent High Priority Bug components

Most helpful comment

Hi all, we cherry picked the fix into Stable and LTS it should be working now. Thanks your patience.

All 15 comments

/cc @zhouyx @micajuine-ho

Hi @claudiorodriguez, thanks for your bug report.

Does throttling the internet speed help to reproduce this bug?

Additionally, if you could attach a screen recording of the bug, that would be very helpful as well.

Hi @micajuine-ho , thanks for responding

Throttling doesn't seem to make much difference

Attaching recording of the issue happening, and how adding the missing CSS rule would make the dialog appear

screen capture amp cmp.webm.zip

@claudiorodriguez In the original screenshot of the bug, I see element style top: calc(32px). Is this something you added?

@micajuine-ho The top calc is an inline style that is applied by amphtml when the iframe is added as a fixed-layer afaik:

https://github.com/ampproject/amphtml/blob/master/src/service/fixed-layer.js#L817-L835

@micajuine-ho We've additionally tried a build were we fired the ready event much earlier than we currently do in case the issue was related to latency but alas the same issue can be replicated.

Thanks for the report, @claudiorodriguez and @hrkhal . We've seen issues like this before on the cache....

I have not been able to reproduce this bug locally. That being said, from the discussion here it seems that something is really strange is happening with the styling.

Within amp-consent, we only ever set --i-amphtml-modal-height together with the missing property:
https://github.com/ampproject/amphtml/blob/b1aa834c53a004a2faf3927a2f59d04a8c8c9cc9/extensions/amp-consent/0.1/consent-ui.js#L710-L713

So I'm not sure why or how this could be happening...

@morsssss Does this problem remind you of anything that's happened on the cache before?

Edit: /cc @ampproject/wg-caching @jridgewell for any ideas

You can force the error in another way that doesn't rely on fast scrolling on the cache. It's certainly not expected user behaviour but more reliably forces the positioning bug.

Repro steps below and video attached.

This comment feels indicative: https://github.com/ampproject/amphtml/blob/master/extensions/amp-consent/0.1/consent-ui.js#L344-L349

We do not see any top property being set on amp-consent when accessing the same amp document directly, only in the cache viewer.

I've just seen issues before where, in the AMP viewer, CSS was introduced that made elements get placed in an undesired vertical position. https://github.com/ampproject/amphtml/issues/17452 is one example.

I see. The scroll position of the page on reload/initialization causing this problem sounds very familiar to https://github.com/ampproject/amphtml/issues/30379. Wondering if there is a way to repro this issue outside a viewer context

@micajuine-ho

WG Caching had a quick look into this. @amaltas tested this specific example and confirmed that Validator doesn't strip the style. Other than that we didn't encounter similar problems recently.

Thank you @MichaelRybak, we've isolated the bug to a fixed layer issue, so no action will be needed from the WG Caching.

Hi all, we cherry picked the fix into Stable and LTS it should be working now. Thanks your patience.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

choumx picture choumx  路  113Comments

jpettitt picture jpettitt  路  77Comments

jpettitt picture jpettitt  路  42Comments

ericlindley-g picture ericlindley-g  路  60Comments

ericlindley-g picture ericlindley-g  路  117Comments