Amphtml: amp-subscription not working on shadow-dom AMP page

Created on 25 Nov 2019  路  15Comments  路  Source: ampproject/amphtml

Amp-Subscription stopped working on iOS13 Simulator and real device and does not trigger any error.

How to test:

  • Load https://rep.repubblica.it page on Safari Mobile iOS 13.
  • Click on "Accedi".
  • Nothing happens on Safari Mobile iOS 13, works fine on iOS 12.

Affected URL (to be tested on Safari Mobile iOS 13): https://rep.repubblica.it/pwa/generale/2019/11/24/news/in_liguria_altro_viadotto_a_rischio_chiudete_o_sara_un_altro_morandi_ecco_l_italia_delle_strade_a_pezzi-241847753/

Non Shadow AMP URL working on the same environment: https://rep.repubblica.it/ws/detail/generale/2019/11/24/news/in_liguria_altro_viadotto_a_rischio_chiudete_o_sara_un_altro_morandi_ecco_l_italia_delle_strade_a_pezzi-241847753/

Screenshot 2019-11-25 at 11 03 45

AMP runtime version: 1911121900560

@iwoak
@nainar

amp-subscriptions Soon Bug access-subscriptions

Most helpful comment

Fixed by #25901. Essentially the newest version of mobile Safari changed how far click events would bubble up. This messed up our click delegation event listener. It appears if a shadowRoot has a body property, the newest version of mobile Safari won't bubble click events up past the body. To fix the bug, we now listen for click events on the body property if it's defined. Otherwise we keep listening to the shadowRoot/doc itself.

All 15 comments

cc @dvoytenko @nainar @jridgewell

I don't think this is an amp-subscriptions issue as we didn't change anything related to this recently. Anybody know if anything else iOS related changed?

We haven't made any changes to Shadow DOM at least for amp-next-page. Confirmed with @wassgha

I can't reproduce this (iphone 11 Pro Mac, ios 13.2 Safari 13 runtime 1911121900560) - both links above work for me.

@jpettitt I can reproduce (are you sure you're clicking on "Accedi" ?

Loading 1911121900560 on an iPhone 7 iOS 13.1, clicking "Accedi" opens a new tab.

Hi Justin, can you confirm that you've tested the Homepage or an article URL like https://rep.repubblica.it/pwa/generale/2019/11/24/news/in_liguria_altro_viadotto_a_rischio_chiudete_o_sara_un_altro_morandi_ecco_l_italia_delle_strade_a_pezzi-241847753/ ?

The Non Shadow AMP version of the articles like https://rep.repubblica.it/ws/detail/generale/2019/11/24/news/in_liguria_altro_viadotto_a_rischio_chiudete_o_sara_un_altro_morandi_ecco_l_italia_delle_strade_a_pezzi-241847753/ works fine to me as well as they are not shadow dom.

I see the issue on "Accedi" not opening the popup on the iOS 13 simulator with the below User Agent but I can't repro anymore on Chrome Mobile iOS UA emulator...

Mozilla/5.0 (iPhone; CPU iPhone OS 13_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0.3 Mobile/15E148 Safari/604.1

Testing https://rep.repubblica.it/pwa/generale/2019/11/24/news/in_liguria_altro_viadotto_a_rischio_chiudete_o_sara_un_altro_morandi_ecco_l_italia_delle_strade_a_pezzi-241847753/ on a real iPhone worked fine. UA is Mozilla/5.0 (iPhone; CPU iPhone OS 13_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0.1 Mobile/15E148 Safari/604.1

I still can repro this on a real iPhone X iOS 13.2.3

Maybe it鈥檚 a bug with 13.2?

Unfortunately all users on 13.2 are affected by this issue and they cannot login on Shadow AMP anymore.

Did you hear or some impactful code changes on Safari that might impact this as we cannot find any error triggered on the console...

I'm able to repro, will investigate further tomorrow once I've got a Macbook :)

Fixed by #25901. Essentially the newest version of mobile Safari changed how far click events would bubble up. This messed up our click delegation event listener. It appears if a shadowRoot has a body property, the newest version of mobile Safari won't bubble click events up past the body. To fix the bug, we now listen for click events on the body property if it's defined. Otherwise we keep listening to the shadowRoot/doc itself.

Thank you Chris!!!

Can you please share with Ivan what Release version may include this fix?

Glad to help!

Hey @iwoak, this fix should be included in the upcoming AMP release version, the one that will launch by Wednesday the 18th, if all goes well. Just to be clear, this release version isn't created or listed on the https://github.com/ampproject/amphtml/releases page yet. I would guess it would appear on that page by Wednesday the 11th though. :smile:

it may seem a small thing but it's enough important for us. Thanks a lot @ChrisAntaki and @gilbertococchi!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

choumx picture choumx  路  3Comments

samanthamorco picture samanthamorco  路  3Comments

edhollinghurst picture edhollinghurst  路  3Comments

cvializ picture cvializ  路  3Comments

choumx picture choumx  路  3Comments