search

ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

[amp-auto-ads] ad resize is failing #31301

Open gurodrigues opened 3 years ago

gurodrigues commented 3 years ago

What's the issue?

amp-auto-ads is not able to resize ads and it's breaking the page when it happens

How do we reproduce the issue?

The error happens most of the time, but not always. In my tests I found out that the i-amphtml-layout-awaiting-size class is breaking the ads because of it's CSS.

URL: https://sample.denakop.com/featured/top-fashion-trends-to-look-for-in-every-important-collection/?amp amp-auto-ads config object: http://jsoneditoronline.org/#left=cloud.a2b84922eb2745f48bc2683afc15ede1 Screenshot: https://imgur.com/a/clN7JYB

What browsers are affected?

All browsers and devices

Which AMP version is affected?

Version: 2011070101001 (current version)

caroqliu commented 3 years ago

cc @ampproject/wg-monetization

gurodrigues commented 3 years ago

@powerivq I was debugging a little more and found that even with fixed size ads this error was happening.

Removing the i-amphtml-layout-awaiting-size class from the amp-ad element would solve the problem in all scenarios that I tested.

https://github.com/ampproject/amphtml/blob/58eb59d5b42a83a23e5b62e8200f57d610b84d71/extensions/amp-auto-ads/0.1/placement.js#L286

https://github.com/ampproject/amphtml/blob/58eb59d5b42a83a23e5b62e8200f57d610b84d71/extensions/amp-auto-ads/0.1/placement.js#L309

I could create a PR with these changes, what do you think?

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.