diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index 56a9c6cbb0eb3..558b1c8c5e896 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -16,7 +16,6 @@ import {Observable} from '../observable'; import {documentStateFor} from '../document-state'; -import {getMode} from '../mode'; import {getServiceForDoc} from '../service'; import {dev} from '../log'; import { @@ -25,7 +24,6 @@ import { parseUrl, removeFragment, } from '../url'; -import {platformFor} from '../platform'; import {timerFor} from '../timer'; import {reportError} from '../error'; import {VisibilityState} from '../visibility-state'; @@ -43,29 +41,6 @@ const SENTINEL_ = '__AMP__'; */ const VIEWER_ORIGIN_TIMEOUT_ = 1000; -/** - * The type of the viewport. - * @enum {string} - */ -export const ViewportType = { - - /** - * Viewer leaves sizing and scrolling up to the AMP document's window. - */ - NATURAL: 'natural', - - /** - * This is AMP-specific type and doesn't come from viewer. This is the type - * that AMP sets when Viewer has requested "natural" viewport on a iOS - * device. - * See: - * https://github.com/ampproject/amphtml/blob/master/spec/amp-html-layout.md - * and {@link ViewportBindingNaturalIosEmbed_} for more details. - */ - NATURAL_IOS_EMBED: 'natural-ios-embed', -}; - - /** * These domains are trusted with more sensitive viewer operations such as * propagating the referrer. If you believe your domain should be here, @@ -136,9 +111,6 @@ export class Viewer { /** @private {number} */ this.prerenderSize_ = 1; - /** @private {!ViewportType} */ - this.viewportType_ = ViewportType.NATURAL; - /** @private {number} */ this.paddingTop_ = 0; @@ -223,22 +195,6 @@ export class Viewer { this.prerenderSize_; dev().fine(TAG_, '- prerenderSize:', this.prerenderSize_); - this.viewportType_ = this.params_['viewportType'] || this.viewportType_; - // Configure scrolling parameters when AMP is iframed on iOS. - const platform = platformFor(this.win); - if (this.viewportType_ == ViewportType.NATURAL && this.isIframed_ && - platform.isIos()) { - this.viewportType_ = ViewportType.NATURAL_IOS_EMBED; - } - // Enable iOS Embedded mode so that it's easy to test against a more - // realistic iOS environment. - if (platform.isIos() && - this.viewportType_ != ViewportType.NATURAL_IOS_EMBED && - (getMode(this.win).localDev || getMode(this.win).development)) { - this.viewportType_ = ViewportType.NATURAL_IOS_EMBED; - } - dev().fine(TAG_, '- viewportType:', this.viewportType_); - this.paddingTop_ = parseInt(this.params_['paddingTop'], 10) || this.paddingTop_; dev().fine(TAG_, '- padding-top:', this.paddingTop_); @@ -645,15 +601,6 @@ export class Viewer { return this.prerenderSize_; } - /** - * See `ViewportType` enum for the set of allowed values. - * See {@link Viewport} and {@link ViewportBinding} for more details. - * @return {!ViewportType} - */ - getViewportType() { - return this.viewportType_; - } - /** * Returns the top padding requested by the viewer. * @return {number} diff --git a/src/service/viewport-impl.js b/src/service/viewport-impl.js index 56388e992fdd7..206a6098f0d7f 100644 --- a/src/service/viewport-impl.js +++ b/src/service/viewport-impl.js @@ -34,7 +34,7 @@ import {installVsyncService} from './vsync-impl'; import {installViewerServiceForDoc} from './viewer-impl'; import {isExperimentOn} from '../experiments'; import {waitForBody} from '../dom'; - +import {getMode} from '../mode'; const TAG_ = 'Viewport'; @@ -1688,7 +1688,7 @@ function createViewport(ampdoc) { const viewer = installViewerServiceForDoc(ampdoc); let binding; if (ampdoc.isSingleDoc() && - viewer.getViewportType() == 'natural-ios-embed') { + getViewportType(ampdoc.win, viewer) == ViewportType.NATURAL_IOS_EMBED) { if (isExperimentOn(ampdoc.win, 'ios-embed-wrapper') // The overriding of document.body fails in iOS7. && platformFor(ampdoc.win).getMajorVersion() > 7) { @@ -1702,6 +1702,46 @@ function createViewport(ampdoc) { return new Viewport(ampdoc, binding, viewer); } +/** + * The type of the viewport. + * @enum {string} + */ +const ViewportType = { + + /** + * Viewer leaves sizing and scrolling up to the AMP document's window. + */ + NATURAL: 'natural', + + /** + * This is AMP-specific type and doesn't come from viewer. This is the type + * that AMP sets when Viewer has requested "natural" viewport on a iOS + * device. + * See: + * https://github.com/ampproject/amphtml/blob/master/spec/amp-html-layout.md + * and {@link ViewportBindingNaturalIosEmbed_} for more details. + */ + NATURAL_IOS_EMBED: 'natural-ios-embed', +}; + +/** + * @param {!Window} win + * @param {!./viewer-impl.Viewer} viewer + * @return {string} + */ +function getViewportType(win, viewer) { + let viewportType = viewer.getParam('viewportType') || ViewportType.NATURAL; + if (platformFor(win).isIos() + && ((viewportType == ViewportType.NATURAL && viewer.isIframed()) + // Enable iOS Embedded mode so that it's easy to test against a more + // realistic iOS environment. + || getMode(win).localDev + || getMode(win).development)) { + viewportType = ViewportType.NATURAL_IOS_EMBED; + } + dev().fine(TAG_, '- viewportType:', viewportType); + return viewportType; +} /** * @param {!./ampdoc-impl.AmpDoc} ampdoc @@ -1710,4 +1750,4 @@ function createViewport(ampdoc) { export function installViewportServiceForDoc(ampdoc) { return /** @type {!Viewport} */ (getServiceForDoc(ampdoc, 'viewport', ampdoc => createViewport(ampdoc))); -}; +} diff --git a/test/functional/test-viewer.js b/test/functional/test-viewer.js index c6d4908881a2a..70f1ccaf37e91 100644 --- a/test/functional/test-viewer.js +++ b/test/functional/test-viewer.js @@ -16,7 +16,6 @@ import {Viewer} from '../../src/service/viewer-impl'; import {dev} from '../../src/log'; -import {platformFor} from '../../src/platform'; import {installDocService} from '../../src/service/ampdoc-impl'; import {installPlatformService} from '../../src/service/platform-impl'; import {installPerformanceService} from '../../src/service/performance-impl'; @@ -36,7 +35,6 @@ describe('Viewer', () => { let clock; let events; let errorStub; - let platform; function changeVisibility(vis) { windowApi.document.hidden = vis !== 'visible'; @@ -89,7 +87,6 @@ describe('Viewer', () => { events = {}; errorStub = sandbox.stub(dev(), 'error'); windowMock = sandbox.mock(windowApi); - platform = platformFor(windowApi); viewer = new Viewer(ampdoc); }); @@ -100,8 +97,7 @@ describe('Viewer', () => { sandbox.restore(); }); - it('should configure as natural viewport by default', () => { - expect(viewer.getViewportType()).to.equal('natural'); + it('should configure as 0 padding top by default', () => { expect(viewer.getPaddingTop()).to.equal(0); }); @@ -109,7 +105,6 @@ describe('Viewer', () => { windowApi.name = '__AMP__viewportType=natural'; windowApi.location.hash = '#paddingTop=17&other=something'; const viewer = new Viewer(ampdoc); - expect(viewer.getViewportType()).to.equal('natural'); expect(viewer.getPaddingTop()).to.equal(17); // All of the startup params are also available via getParam. @@ -340,27 +335,6 @@ describe('Viewer', () => { expect(send.callCount).to.equal(0); }); - it('should configure correctly for iOS embedding', () => { - windowApi.name = '__AMP__viewportType=natural'; - windowApi.parent = {}; - sandbox.mock(platform).expects('isIos').returns(true).atLeast(1); - const viewer = new Viewer(ampdoc); - - expect(viewer.getViewportType()).to.equal('natural-ios-embed'); - }); - - it('should NOT configure for iOS embedding if not embedded', () => { - windowApi.name = '__AMP__viewportType=natural'; - windowApi.parent = windowApi; - sandbox.mock(platform).expects('isIos').returns(true).atLeast(1); - windowApi.AMP_MODE = { - localDev: false, - development: false, - }; - const viewportType = new Viewer(ampdoc).getViewportType(); - expect(viewportType).to.equal('natural'); - }); - it('should receive viewport event', () => { let viewportEvent = null; viewer.onViewportEvent(event => { diff --git a/test/functional/test-viewport.js b/test/functional/test-viewport.js index d5b103b124523..c30520232c597 100644 --- a/test/functional/test-viewport.js +++ b/test/functional/test-viewport.js @@ -16,6 +16,7 @@ import {AmpDocSingle, installDocService} from '../../src/service/ampdoc-impl'; import { + installViewportServiceForDoc, Viewport, ViewportBindingDef, ViewportBindingIosEmbedWrapper_, @@ -1550,3 +1551,57 @@ describes.realWin('ViewportBindingIosEmbedWrapper', {ampCss: true}, env => { }); }); }); + +describe('createViewport', () => { + + describes.fakeWin('in Android', {win: {navigator: {userAgent: 'Android'}}}, + env => { + let win; + + beforeEach(() => { + win = env.win; + installPlatformService(win); + installTimerService(win); + }); + + it('should bind to "natural" when not iframed', () => { + win.parent = win; + const ampDoc = installDocService(win, true).getAmpDoc(); + const viewport = installViewportServiceForDoc(ampDoc); + expect(viewport.binding_).to.be.instanceof(ViewportBindingNatural_); + }); + + it('should bind to "naturual" when iframed', () => { + win.parent = {}; + const ampDoc = installDocService(win, true).getAmpDoc(); + const viewport = installViewportServiceForDoc(ampDoc); + expect(viewport.binding_).to.be.instanceof(ViewportBindingNatural_); + }); + }); + + describes.fakeWin('in iOS', {win: {navigator: {userAgent: 'iPhone'}}}, + env => { + let win; + + beforeEach(() => { + win = env.win; + installPlatformService(win); + installTimerService(win); + }); + + it('should bind to "natural" when not iframed', () => { + win.parent = win; + const ampDoc = installDocService(win, true).getAmpDoc(); + const viewport = installViewportServiceForDoc(ampDoc); + expect(viewport.binding_).to.be.instanceof(ViewportBindingNatural_); + }); + + it('should bind to "natural iOS embed" when iframed', () => { + win.parent = {}; + const ampDoc = installDocService(win, true).getAmpDoc(); + const viewport = installViewportServiceForDoc(ampDoc); + expect(viewport.binding_).to + .be.instanceof(ViewportBindingNaturalIosEmbed_); + }); + }); +});