Skip to content

Commit

Permalink
Use manifest and origin-manifest to look for banner source (#6388)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mkhatib
mkhatib committed Nov 29, 2016
1 parent 9f686eb commit b652d1c
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 35 deletions.
2 changes: 1 addition & 1 deletion examples/amp-fresh.amp.html
View file
Expand Up @@ -11,7 +11,7 @@
<script async custom-element="amp-fresh" src="https://cdn.ampproject.org/v0/amp-fresh-0.1.js"></script>
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
<link rel="amp-manifest" href="medium-manifest.json">
<link rel="manifest" href="medium-manifest.json">
<script async src="./viewer-integr.js" data-amp-report-test="viewer-integr.js"></script>
</head>
<body>
Expand Down
2 changes: 1 addition & 1 deletion examples/article-fixed-header.amp.html
View file
Expand Up @@ -238,7 +238,7 @@
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
<meta name="apple-itunes-app" content="app-id=828256236, app-argument=medium://p/cb7f223fad86">
<link rel="amp-manifest" href="medium-manifest.json">
<link rel="manifest" href="medium-manifest.json">
<script async src="https://cdn.ampproject.org/viewer/google/v5.js"></script>
</head>
<body>
Expand Down
2 changes: 1 addition & 1 deletion examples/article.amp.html
View file
Expand Up @@ -219,7 +219,7 @@
<script async custom-element="amp-app-banner" src="https://cdn.ampproject.org/v0/amp-app-banner-0.1.js" data-amp-report-test="amp-app-banner.js"></script>
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<meta name="apple-itunes-app" content="app-id=828256236, app-argument=medium://p/cb7f223fad86">
<link rel="amp-manifest" href="medium-manifest.json">
<link rel="manifest" href="medium-manifest.json">
<script async src="./viewer-integr.js" data-amp-report-test="viewer-integr.js"></script>
</head>
<body>
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-app-banner/0.1/amp-app-banner.js
View file
Expand Up @@ -339,7 +339,7 @@ export class AmpAndroidAppBanner extends AbstractAppBanner {

const viewer = viewerForDoc(this.getAmpDoc());
this.manifestLink_ = this.win.document.head.querySelector(
'link[rel=manifest],link[rel=amp-manifest]');
'link[rel=manifest],link[rel=origin-manifest]');

const platform = platformFor(this.win);
// We want to fallback to browser builtin mechanism when possible.
Expand Down
86 changes: 56 additions & 30 deletions extensions/amp-app-banner/0.1/test/test-amp-app-banner.js
View file
Expand Up @@ -97,13 +97,15 @@ describe('amp-app-banner', () => {
iframe.doc.head.appendChild(meta);
}

if (config.manifest) {
const manifestObj = config.originManifest || config.manifest;
if (manifestObj) {
const rel = config.originManifest ? 'origin-manifest' : 'manifest';
const manifest = iframe.doc.createElement('link');
manifest.setAttribute('rel', 'amp-manifest');
manifest.setAttribute('href', config.manifest.href);
manifest.setAttribute('rel', rel);
manifest.setAttribute('href', manifestObj.href);
iframe.doc.head.appendChild(manifest);
sandbox.mock(xhrFor(iframe.win)).expects('fetchJson')
.returns(Promise.resolve(config.manifest.content));
.returns(Promise.resolve(manifestObj.content));
}

const banner = iframe.doc.createElement('amp-app-banner');
Expand Down Expand Up @@ -153,6 +155,42 @@ describe('amp-app-banner', () => {
});
}

function testManifestPreconnectPreload(rel) {
const config = {};
config[rel] = manifest;
return () => {
return getAppBanner(config).then(banner => {
const impl = banner.implementation_;
sandbox.stub(impl.preconnect, 'url');
sandbox.stub(impl.preconnect, 'preload');
impl.preconnectCallback(true);
expect(impl.preconnect.url.called).to.be.true;
expect(impl.preconnect.url.callCount).to.equal(1);
expect(impl.preconnect.url.calledWith('https://play.google.com'))
.to.be.true;
expect(impl.preconnect.preload.called).to.be.true;
expect(impl.preconnect.preload.callCount).to.equal(1);
expect(impl.preconnect.preload.calledWith(
'https://example.com/manifest.json')).to.be.true;
});
};
}

function testManifestParseAndHrefs(rel) {
const config = {};
config[rel] = manifest;
return () => {
sandbox.spy(AbstractAppBanner.prototype, 'setupOpenButton_');
return getAppBanner({manifest}).then(el => {
expect(AbstractAppBanner.prototype.setupOpenButton_.calledWith(
el.querySelector('button[open-button]'),
'android-app://com.medium.reader/https/example.com/amps.html',
'https://play.google.com/store/apps/details?id=com.medium.reader'
)).to.be.true;
});
};
}

beforeEach(() => {
sandbox = sinon.sandbox.create();
platform = platformFor(window);
Expand Down Expand Up @@ -261,22 +299,10 @@ describe('amp-app-banner', () => {
isChrome = false;
});

it('should preconnect to play store and preload manifest', () => {
return getAppBanner({manifest}).then(banner => {
const impl = banner.implementation_;
sandbox.stub(impl.preconnect, 'url');
sandbox.stub(impl.preconnect, 'preload');
impl.preconnectCallback(true);
expect(impl.preconnect.url.called).to.be.true;
expect(impl.preconnect.url.callCount).to.equal(1);
expect(impl.preconnect.url.calledWith('https://play.google.com'))
.to.be.true;
expect(impl.preconnect.preload.called).to.be.true;
expect(impl.preconnect.preload.callCount).to.equal(1);
expect(impl.preconnect.preload.calledWith(
'https://example.com/manifest.json')).to.be.true;
});
});
it('should preconnect to play store and preload manifest',
testManifestPreconnectPreload('manifest'));
it('should preconnect to play store and preload origin-manifest',
testManifestPreconnectPreload('originManifest'));

it('should throw if open button is missing', testButtonMissing);
it('should add dismiss button and update padding', testAddDismissButton);
Expand All @@ -288,23 +314,23 @@ describe('amp-app-banner', () => {
});
});

it('should remove banner if origin-manifest is not provided', () => {
return getAppBanner({originManifest: null}).then(banner => {
expect(banner.parentElement).to.be.null;
});
});

it('should remove banner if chrome', () => {
isChrome = true;
return getAppBanner().then(banner => {
expect(banner.parentElement).to.be.null;
});
});

it('should parse manifest and set hrefs', () => {
sandbox.spy(AbstractAppBanner.prototype, 'setupOpenButton_');
return getAppBanner({manifest}).then(el => {
expect(AbstractAppBanner.prototype.setupOpenButton_.calledWith(
el.querySelector('button[open-button]'),
'android-app://com.medium.reader/https/example.com/amps.html',
'https://play.google.com/store/apps/details?id=com.medium.reader'
)).to.be.true;
});
});
it('should parse manifest and set hrefs',
testManifestParseAndHrefs('manifest'));
it('should parse manifest and set hrefs',
testManifestParseAndHrefs('originManifest'));
});

describe('Abstract App Banner', () => {
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-app-banner/amp-app-banner.md
View file
Expand Up @@ -171,7 +171,7 @@ Not permitted: **disabled**
<head>
<meta name="apple-itunes-app"
content="app-id=123456789, app-argument=app-name://link/to/app-content">
<link rel="amp-manifest" href="https://link/to/manifest.json">
<link rel="manifest" href="https://link/to/manifest.json">
</head>

. . .
Expand Down

0 comments on commit b652d1c

Please sign in to comment.