From 7038ac6b5f221b0ea1ba8b182c90dc94b8669f86 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Wed, 13 Apr 2016 11:23:04 -0700 Subject: [PATCH] Revert "Remove ad during unlayout" (#2898) This reverts commit 03850ee84f6e548392b93f02e359bed11f60c9ec. We suspect that this negatively affects UX by unloading during accidental swipes. Rollforward will improve heuristic. --- builtins/amp-ad.js | 27 +------------------------ extensions/amp-iframe/0.1/amp-iframe.js | 2 -- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/builtins/amp-ad.js b/builtins/amp-ad.js index 33cffd9c1b3d..53fc41f635a9 100644 --- a/builtins/amp-ad.js +++ b/builtins/amp-ad.js @@ -28,7 +28,6 @@ import {timer} from '../src/timer'; import {user} from '../src/log'; import {userNotificationManagerFor} from '../src/user-notification'; import {viewerFor} from '../src/viewer'; -import {removeElement} from '../src/dom'; /** @private @const These tags are allowed to have fixed positioning */ @@ -285,30 +284,6 @@ export function installAd(win) { return loadPromise(this.iframe_); } - /** @override */ - unlayoutCallback() { - if (this.iframe_) { - removeElement(this.iframe_); - if (this.placeholder_) { - this.togglePlaceholder(true); - } - if (this.fallback_) { - this.toggleFallback(false); - } - - this.iframe_ = null; - // IntersectionObserver's listeners were cleaned up by - // setInViewport(false) before #unlayoutCallback - this.intersectionObserver_ = null; - } - return true; - } - - /** @override */ - unlayoutOnPause() { - return true; - } - /** * @return {!Promise} A promise for a CID or undefined if * - the ad network does not request one or @@ -418,7 +393,7 @@ export function installAd(win) { } // Remove the iframe only if it is not the master. if (this.iframe_.name.indexOf('_master') == -1) { - removeElement(this.iframe_); + this.element.removeChild(this.iframe_); this.iframe_ = null; } }); diff --git a/extensions/amp-iframe/0.1/amp-iframe.js b/extensions/amp-iframe/0.1/amp-iframe.js index 668c86cb3d58..cdbadf6a6794 100644 --- a/extensions/amp-iframe/0.1/amp-iframe.js +++ b/extensions/amp-iframe/0.1/amp-iframe.js @@ -339,8 +339,6 @@ export class AmpIframe extends AMP.BaseElement { } this.iframe_ = null; - // IntersectionObserver's listeners were cleaned up by - // setInViewport(false) before #unlayoutCallback this.intersectionObserver_ = null; } return true;