Skip to content

Commit

Permalink
Revert "Remove ad during unlayout" (#2898)
Browse files Browse the repository at this point in the history
This reverts commit 03850ee.

We suspect that this negatively affects UX by unloading during accidental swipes. Rollforward will improve heuristic.
  • Loading branch information
cramforce authored and erwinmombay committed Apr 13, 2016
1 parent d39f7e3 commit 7038ac6
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 28 deletions.
27 changes: 1 addition & 26 deletions builtins/amp-ad.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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<string|undefined>} A promise for a CID or undefined if
* - the ad network does not request one or
Expand Down Expand Up @@ -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;
}
});
Expand Down
2 changes: 0 additions & 2 deletions extensions/amp-iframe/0.1/amp-iframe.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 7038ac6

Please sign in to comment.