Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change.intersectionRect is never updated #29759

Closed
ruizb opened this issue Aug 10, 2020 · 11 comments · Fixed by #29853
Closed

change.intersectionRect is never updated #29759

ruizb opened this issue Aug 10, 2020 · 11 comments · Fixed by #29853
Assignees

Comments

@ruizb
Copy link

ruizb commented Aug 10, 2020

What's the issue?

The change.intersectionRect object is never updated.

How do we reproduce the issue?

  1. Resize your viewport to be e.g. 850px wide (not too wide since there isn't much content on the page).

  2. Go to https://amp.teads.tv/.

  3. Wait a little bit (1-2 seconds), then scroll down the page.

    See demo

    amp-nightly-no-startad

  4. Below the 2nd "lorem ipsum" paragraph, you should see a light-grey rectangle. This is where the ad is supposed to play. On stable (v. 2007242032002) and beta (v. 2007302351001), the ad is indeed displayed.

  5. If you take the focus of the amp-ad > iframe > body element, then observe the changes on _teads_amp.change.intersectionRect, you'll notice that the values are never updated. We are using this object to detect when the ad is visible, then start/play it. Since the object contains only zeros, we don't consider the ad visible, and we don't start it.

    See changes observed on v. 2007242032002 (stable)

    amp-stable-intersectionrect-observed

    See changes observed on v. 2008072132000 (nightly)

    amp-nightly-intersectionrect-observed

What browsers are affected?

So far, I could test only on these browsers:

  • Chrome v84, both desktop and mobile
  • Firefox v79

I couldn't set the nightly version on Safari v13.1.

Which AMP version is affected?

2008072132000

@ruizb
Copy link
Author

ruizb commented Aug 11, 2020

Additionally, when using context.observeIntersection(callback), the callback doesn't seem to be triggered anymore when scrolling in the page. It works on 2007242032002 but not on 2008072132000 anymore.

@ruizb
Copy link
Author

ruizb commented Aug 17, 2020

It doesn't work with the current Experimental version, v. 2008072132002.

@ruizb
Copy link
Author

ruizb commented Aug 17, 2020

I didn't want to ping anyone, but I couldn't get any answer since last week, and this issue prevents all our ads from being started. Since it's been promoted to the Experimental channel, I'm a bit worried.

I'm pretty sure it's due to the following commit: c720e08. Please @choumx, can you have a look?

@dreamofabear
Copy link

Thanks for the detailed report. I can repro the issue, but I don't think it's c720e08 because AMP.toggleExperiment('intersect-resources') doesn't fix it. If I refresh the page with the ad in viewport, then the ad loads as expected.

@ampproject/wg-ads-reviewers

@ruizb
Copy link
Author

ruizb commented Aug 17, 2020

Hello, thanks for the reply!

Yes, the initial intersectionRect has the correct values, so when the ad is in the viewport when loading the page, it plays. However, as the rect is never updated, the ad continues to play despite leaving the viewport (it's supposed to be paused when "not visible").

Our automated tests stopped working since a commit merged on 2020-08-06 or 2020-08-07. I thought it was this one since it's about "intersect", but I guess I was wrong!

@dreamofabear
Copy link

Yea it's somewhere in here: 2007302...2008070

@dreamofabear
Copy link

Tried git bisect locally on unminified builds and I think it's #28929.

@lannka
Copy link
Contributor

lannka commented Aug 17, 2020

Thanks Will, I can take it from there

@kristoferbaxter
Copy link
Contributor

For clarity @ruizb, @lannka addressed this regression in #29853. We're following up with some additional tests for confidence going forward.

@ruizb
Copy link
Author

ruizb commented Aug 18, 2020

Hey, thanks for the fix! I tested on the latest nightly channel (v. 2008180007000) and it works fine! 👍

@ruizb
Copy link
Author

ruizb commented Aug 19, 2020

Hello again! It works, but it takes much more time to "start working" (v. 2008180007000). I noticed the context.observeIntersection callback was called a few times, then paused for some time, then got called again every time I scroll. I'm not sure what could introduce some delay? (we're talking about several seconds)

Demo v. 2008180007000

CleanShot 2020-08-19 at 09 59 23

FTR, on the current stable version (v. 2007302351001), there is no delay.

Demo v. 2007302351001

CleanShot 2020-08-19 at 10 02 25

I tested on Chrome Mac OS so far, and our automated tests on Safari 12.0 are flaky (I suspect this delay to be the cause).

EDIT I actually created another issue since this problem occurs with the Beta channel as well: #29893.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants