Amphtml: Doubleclick served a4a ads do not work when loaded in a PWA with shadowdom

Created on 10 Oct 2019  Â·  19Comments  Â·  Source: ampproject/amphtml

What's the issue?

When loading an AMP document into a shadowdoc as part of PWA, should the document contain an amp-ad of type "doubleclick" which loads an amp4ads based ad, the ad fails to load due to numerous errors related to No ampdoc found for [object HTMLHtmlElement].

image

How do we reproduce the issue?

A demo is available here: https://d1kvlucx6m02nr.cloudfront.net/

Upon first ( uncached ) load the ad loads correctly as the document is yet to be loaded within the shell container.

image

However as soon as navigation occurs & the shell container is used the ad fail to load.

  1. Navigate to https://d1kvlucx6m02nr.cloudfront.net/
  2. Click on "Article 1"

What browsers are affected?

All browsers

Which AMP version is affected?

HTML shadows – Version 1910071803120

Based on previous issues related to PWA, shadowdoc & Doubleclick it looks like there may have a been a regression.

Additionally the a4a ad example loaded via projects example of a PWA is also broken. http://localhost:8000/pwa/

Bug

All 19 comments

Just wondering whether this issue is on anyones radar in @ampproject/wg-ads

@calebcordry Would you mind taking a quick look?

Thanks @jeffjose & @calebcordry. FYI I had to replace the ad as it is no-longer being trafficked. I've replaced it with the a4a example.

I've looked at the demo page myself in the latest Chrome Canary - it behaves in the same way as the screenshot posted here. It's behaving in the same fashion in Safari on OSX, so I can rule this out as not being a browser-specific quirk.

@calebcordry could you estimate when you would be able to put some headspace around this issue or find someone else on the AMP team that could?

@indieisaconcept @mehigh @derekherman Sorry for the delay.

@indieisaconcept Thanks a lot for the example page.

Did some searching and I think this might be related to the work in #22734 @dvoytenko would you mind taking a look?

@calebcordry thanks. Happy to provide further support/examples as required to assist with getting this resolved.

@calebcordry The #22734 is not launched yet. And based on error stacks I can confirm that this is not in the same codepath.

@dvoytenko thanks for checking. I will dig more into this today.

cc/ @ampproject/wg-ads

@calebcordry I had chance to pull down your branch. It looks like the change your proposing in #25321 resolves the issue which is great news.

@calebcordry great to see this merged. When do you think this could be feasibly released?

@indianburger @indieisaconcept While debugging this issue we also found a related issue fixed in #25357. We are currently discussing the risk/reward in trying a cherry pick. I will update here with the resolution. Worst case it would follow the normal AMP release schedule.

Cherrypick is in #25367, so hopefully it can be in PROD sooner.

@indieisaconcept Change should now be live in canary. You can opt in at https://cdn.ampproject.org/experiments.html . I tested the new version on you example page, but feel free to test on your own, and let us know if you run into anything unexpected.

@calebcordry thanks! I can confirm based on testing that I am seeing the same behaviour as yourself with the example page & when testing against our pre-prod environment I am seeing ads load correctly.

@calebcordry, @dvoytenko just wanted to say thanks for getting this resolved so quickly. I verified again today following the prod release.

@indieisaconcept Thanks for reporting and helping us repro this!

Thanks for the quick turnaround on this, @calebcordry and @dvoytenko!

Was this page helpful?
0 / 5 - 0 ratings