Skip to content

Commit

Permalink
Revert #5960 which is a revert of #5887 (Clean up: Move viewport spec…
Browse files Browse the repository at this point in the history 
…ific logic out from viewer.) (#5972)
  • Loading branch information
@lannka
lannka committed Nov 2, 2016
1 parent 0fc1aca commit 08679ec
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 83 deletions.
53 changes: 0 additions & 53 deletions src/service/viewer-impl.js
View file
Expand Up @@ -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 {
Expand All @@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -136,9 +111,6 @@ export class Viewer {
/** @private {number} */
this.prerenderSize_ = 1;

/** @private {!ViewportType} */
this.viewportType_ = ViewportType.NATURAL;

/** @private {number} */
this.paddingTop_ = 0;

Expand Down Expand Up @@ -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_);
Expand Down Expand Up @@ -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}
Expand Down
46 changes: 43 additions & 3 deletions src/service/viewport-impl.js
View file
Expand Up @@ -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';

Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -1710,4 +1750,4 @@ function createViewport(ampdoc) {
export function installViewportServiceForDoc(ampdoc) {
return /** @type {!Viewport} */ (getServiceForDoc(ampdoc, 'viewport',
ampdoc => createViewport(ampdoc)));
};
}
28 changes: 1 addition & 27 deletions test/functional/test-viewer.js
View file
Expand Up @@ -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';
Expand All @@ -36,7 +35,6 @@ describe('Viewer', () => {
let clock;
let events;
let errorStub;
let platform;

function changeVisibility(vis) {
windowApi.document.hidden = vis !== 'visible';
Expand Down Expand Up @@ -89,7 +87,6 @@ describe('Viewer', () => {
events = {};
errorStub = sandbox.stub(dev(), 'error');
windowMock = sandbox.mock(windowApi);
platform = platformFor(windowApi);
viewer = new Viewer(ampdoc);
});

Expand All @@ -100,16 +97,14 @@ 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);
});

it('should configure correctly based on window name and hash', () => {
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.
Expand Down Expand Up @@ -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 => {
Expand Down
55 changes: 55 additions & 0 deletions test/functional/test-viewport.js
View file
Expand Up @@ -16,6 +16,7 @@

import {AmpDocSingle, installDocService} from '../../src/service/ampdoc-impl';
import {
installViewportServiceForDoc,
Viewport,
ViewportBindingDef,
ViewportBindingIosEmbedWrapper_,
Expand Down Expand Up @@ -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_);
});
});
});

0 comments on commit 08679ec

Please sign in to comment.