Skip to content

Conversation

@zhouyx
Copy link
Contributor

@zhouyx zhouyx commented Aug 4, 2016

No description provided.

// setInViewport(false) before #unlayoutCallback
this.intersectionObserver_.destroy();
this.intersectionObserver_ = null;
if (this.intersectionObserver_) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check here, it's guaranteed if iframe_ is present.

@jridgewell
Copy link
Contributor

Looks like <amp-iframe> is the only one that needs this check. LGTM afterwards.

@jridgewell
Copy link
Contributor

Fixes #4372.

@zhouyx
Copy link
Contributor Author

zhouyx commented Aug 4, 2016

you're right. amp-ad will create intersectionObserver anyway. removed check for ad

@zhouyx zhouyx merged commit 9d77526 into ampproject:master Aug 4, 2016
ariangibson pushed a commit to Mixpo/amphtml that referenced this pull request Sep 7, 2016
@zhouyx zhouyx deleted the fix-destory branch September 28, 2016 17:23
mityaha pushed a commit to brightcove-archive/ooyala_amphtml that referenced this pull request Nov 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants