Amphtml: NYTimes: Account linking confirmation overlaying subscription message

[franniehannan]

What's the issue?

We're working on adding a new entitlement for NYtimes registered users, which would give them access to more articles. Because these users are not subscribers, they still get the standard subscription messaging after they log in. However, the account linking confirmation shows up _behind_ the subscription message and therefore users can't dismiss it. We've tried a bunch of things on our end with the z index of the message, but nothing has worked. We'd like for the Google linking confirmation to show up on top of anything else present on the AMP page, and that will resolve the issue. We need to resolve this before doing any kind of registration testing on AMP.

How do we reproduce the issue?

1. Open a New York Times AMP page on a mobile device (note, this isn't reproducible in a mobile simulator). You also need to be logged in to a Google account with no linked NYTimes subscriptions. Example URL: https://www.google.com/amp/s/www.nytimes.com/2019/09/04/technology/google-youtube-fine-ftc.amp.html

2. Click the "log in" link in the top right corner

3. Log in using one of these test accounts:
   93044123 - [email protected]
   93044135 - [email protected]
   93044140 - [email protected]
   93044148 - [email protected]
   93044151 - [email protected]
   93044160 - [email protected]
   93044165 - [email protected]
   93044169 - [email protected]
   93044171 - [email protected]
   93044175 - [email protected]

password is test123

1. Observe linking confirmation is displayed underneath the message to subscribe.
   

What browsers are affected?

Mobile Chrome and Safari

Which AMP version is affected?

Not sure.

[jpettitt]

Does this happen on iOS? Android? both? (it will help as fixed layers are currently handles slightly differently on each)

[jpettitt]

I can reproduce this, it's an artifact of the way iOS layering is managed due to some bugs in the way scrolling is handled in iframes. I'll see if there is an easy fix. Note this will go away with IOS13 which doesn't need the scrolling fix.

[jpettitt]

Proposed fix:

Expose FixedLayer.enterLightbox() and FixedLayer.leaveLightbox() in viewport-impl then call them from dialog.js on open and close. This will change the transfer layer visibility when our dialog needs to be on top.

The existing enterLightboxMode() function isn't suitable because it messages the viewer to hide the titlebar.

cc @dvoytenko

[dvoytenko]

/cc @lannka

This is possible and might be the most performant. Not sure what to call this API in Viewport service. Maybe as simple as "showOverlayDialog(function():Promise)"? This way we will wrap any overlay dialog display functionality inside such a call to avoid UI interference?

[lannka]

it's a bit unclear what those methods do, are we able to consolidate?

• viewport.enterLightboxMode
• viewport.enterOverlayMode
• fixedLayer.enterLightbox
• binding.updateLightboxMode

Note that viewport service now has a separate implementation for inabox, concise interfaces will be ideal.

[jpettitt]

Found a cleaner solution - I'll just use addToFixedLayer() to put the SwG dialog on the fixed layer

[jpettitt]

On further investigation none of the above will fix it. To correctly fix it we need to make the swg.js iframes visible to the AMPDoc.

[jhoeltzel]

@jpettitt, wanted to check - is your suggestion (making iframes visible to the AMPDoc) something that will pursue or is this on hold for now? Thank your for looking into it!

[jpettitt]

We merged a fix as part of #24440 this morning - it will be in the Canary / Dev Channel next Tuesday late in the day.

The final fix was to add addToFixedLayer() to the Doc interface SwG uses as a no-op on non a AMP but when merged into AMP we pass in one that hooks up to AMP's Viewport which manager the iOS scrolling bug mitigation that underlies the need for a separate fixed layer.

The effect of all that is to make all SwG fixed elements visible to the AMP layer manager which puts them in the same stacking context and avoid the stacking order issue.

[This whole thing is because webkit on iOS < 13.0 doesn't smooth scroll iframes that have fixed elements in their <body>. The workaround is to create a second <body> and migrate the fixed elements to it. Ugly but it works. Fixed in iOS 13]

[jpettitt]

@franniehannan @jhoeltzel you should probably kill those test accounts since the password is in this issue.