Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't ssr amp-img in stories #896

Merged
merged 1 commit into from
Aug 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions packages/optimizer/lib/AmpConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,27 @@ module.exports = {
}
return false;
},
isAmpStory: (head) => {
for (const node of head.children) {
if (isAmpScriptImport(node) && node.attribs['custom-element'] === 'amp-story') {
return true;
}
}
return false;
},
};

function isAmpScriptImport(scriptNode) {
if (scriptNode.tagName !== 'script') {
return false;
}
if (!scriptNode.attribs) {
return false;
}
const customElement =
scriptNode.attribs['custom-element'] || scriptNode.attribs['custom-template'] || '';
if (!customElement.startsWith('amp-')) {
return false;
}
return true;
}
11 changes: 8 additions & 3 deletions packages/optimizer/lib/transformers/PreloadHeroImage.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const {
} = require('../NodeUtils');
const {findMetaViewport, skipNodeAndChildren} = require('../HtmlDomHelper');
const {isValidImageSrcURL} = require('../URLUtils');
const {isTemplate} = require('../AmpConstants');
const amphtml = require('../AmpConstants');

// Images smaller than 150px are considered tiny
const TINY_IMG_THRESHOLD = 150;
Expand Down Expand Up @@ -72,10 +72,15 @@ class PreloadHeroImage {
);
heroImageCount = DATA_HERO_MAX;
}
const isAmpStory = amphtml.isAmpStory(head);
for (let i = 0; i < heroImageCount; i++) {
const heroImage = heroImages[i];
this.generatePreload(heroImage, head, referenceNode);
this.generateImg(heroImage.ampImg);
// AMP Stories don't support ssr'd amp-img yet
// See https://github.com/ampproject/amphtml/issues/29850
if (!isAmpStory) {
this.generateImg(heroImage.ampImg);
}
}
}

Expand Down Expand Up @@ -132,7 +137,7 @@ class PreloadHeroImage {
if (!heroImageCandidate && heroImages.length === 0) {
heroImageCandidate = this.isCandidateHeroImage(node);
}
if (isTemplate(root)) {
if (amphtml.isTemplate(root)) {
// Ignore images inside templates
node = skipNodeAndChildren(node);
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<!doctype html>
<html amp lang="en" i-amphtml-layout transformed="self;v=1">
<head>
<meta charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<link rel="preload" href="https://cdn.ampproject.org/v0.js" as="script">
<script async src="https://cdn.ampproject.org/v0.js"></script>
<script async custom-element="amp-story" src="https://cdn.ampproject.org/v0/amp-story-1.0.js"></script>
<link rel="preload" href="https://amp.dev/static/samples/img/story_dog2.jpg" as="image" data-hero>
<title>My Story</title>
<link rel="canonical" href="helloworld.html"><style amp-custom>body {
font-family: 'Roboto', sans-serif;
}
amp-story-page {
background: white;
}
h1 {
font-size: 2.875em;
font-weight: normal;
line-height: 1.174;
text-transform: uppercase;
}</style><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>
</head>
<body>
<amp-story title="test" publisher="test" poster-portrait-src="test" publisher-logo-src="img.jpg" standalone class="i-amphtml-layout-container" i-amphtml-layout="container">
<amp-story-page id="cover" class="i-amphtml-layout-container" i-amphtml-layout="container">
<amp-story-grid-layer template="vertical" class="i-amphtml-layout-container" i-amphtml-layout="container">
<h1>Hello World</h1>
<p>This is the cover page of this story.</p>
<amp-img data-hero src="https://amp.dev/static/samples/img/story_dog2.jpg" width="300" height="300" layout="responsive" alt class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
<i-amphtml-sizer style="display:block;padding-top:100.0000%;"></i-amphtml-sizer>
</amp-img>
</amp-story-grid-layer>
</amp-story-page>
</amp-story>
</body>
</html>
37 changes: 37 additions & 0 deletions packages/optimizer/spec/end-to-end/story/expected_output.lts.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<!doctype html>
<html amp lang="en" i-amphtml-layout transformed="self;v=1">
<head>
<meta charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<link rel="preload" href="https://cdn.ampproject.org/lts/v0.js" as="script">
<script async src="https://cdn.ampproject.org/lts/v0.js"></script>
<script async custom-element="amp-story" src="https://cdn.ampproject.org/lts/v0/amp-story-1.0.js"></script>
<link rel="preload" href="https://amp.dev/static/samples/img/story_dog2.jpg" as="image" data-hero>
<title>My Story</title>
<link rel="canonical" href="helloworld.html"><style amp-custom>body {
font-family: 'Roboto', sans-serif;
}
amp-story-page {
background: white;
}
h1 {
font-size: 2.875em;
font-weight: normal;
line-height: 1.174;
text-transform: uppercase;
}</style><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>
</head>
<body>
<amp-story title="test" publisher="test" poster-portrait-src="test" publisher-logo-src="img.jpg" standalone class="i-amphtml-layout-container" i-amphtml-layout="container">
<amp-story-page id="cover" class="i-amphtml-layout-container" i-amphtml-layout="container">
<amp-story-grid-layer template="vertical" class="i-amphtml-layout-container" i-amphtml-layout="container">
<h1>Hello World</h1>
<p>This is the cover page of this story.</p>
<amp-img data-hero src="https://amp.dev/static/samples/img/story_dog2.jpg" width="300" height="300" layout="responsive" alt class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
<i-amphtml-sizer style="display:block;padding-top:100.0000%;"></i-amphtml-sizer>
</amp-img>
</amp-story-grid-layer>
</amp-story-page>
</amp-story>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!doctype html>
<html lang="en" i-amphtml-layout transformed="self;v=1">
<head>
<meta charset="utf-8"><style amp-runtime i-amphtml-version="123456789000000">/* ampproject.org/rtv v0.css */amp-img[i-amphtml-ssr]:not(.i-amphtml-element):not([layout=container])>*{display: block;}</style>
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<link rel="preload" href="https://cdn.ampproject.org/v0.js" as="script">
<script async src="https://cdn.ampproject.org/v0.js"></script>
<script async custom-element="amp-story" src="https://cdn.ampproject.org/v0/amp-story-1.0.js"></script>
<link rel="preload" href="https://amp.dev/static/samples/img/story_dog2.jpg" as="image" data-hero>
<title>My Story</title>
<link rel="canonical" href="helloworld.html"><style amp-custom>body {
font-family: 'Roboto', sans-serif;
}
amp-story-page {
background: white;
}
h1 {
font-size: 2.875em;
font-weight: normal;
line-height: 1.174;
text-transform: uppercase;
}</style>
<link rel="amphtml" href="https://example.com/amp-version.html"><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>
</head>
<body>
<amp-story title="test" publisher="test" poster-portrait-src="test" publisher-logo-src="img.jpg" standalone class="i-amphtml-layout-container" i-amphtml-layout="container">
<amp-story-page id="cover" class="i-amphtml-layout-container" i-amphtml-layout="container">
<amp-story-grid-layer template="vertical" class="i-amphtml-layout-container" i-amphtml-layout="container">
<h1>Hello World</h1>
<p>This is the cover page of this story.</p>
<amp-img data-hero src="https://amp.dev/static/samples/img/story_dog2.jpg" width="300" height="300" layout="responsive" alt class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
<i-amphtml-sizer style="display:block;padding-top:100.0000%;"></i-amphtml-sizer>
</amp-img>
</amp-story-grid-layer>
</amp-story-page>
</amp-story>
</body>
</html>
39 changes: 39 additions & 0 deletions packages/optimizer/spec/end-to-end/story/input.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!doctype html>
<html amp lang="en">
<head>
<meta charset="utf-8">
<script async src="https://cdn.ampproject.org/v0.js"></script>
<script async custom-element="amp-story" src="https://cdn.ampproject.org/v0/amp-story-1.0.js"></script>
<title>My Story</title>
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<link rel="canonical" href="helloworld.html">
<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>
<style amp-custom>
body {
font-family: 'Roboto', sans-serif;
}
amp-story-page {
background: white;
}
h1 {
font-size: 2.875em;
font-weight: normal;
line-height: 1.174;
text-transform: uppercase;
}
</style>
</head>

<body>

<amp-story title="test" publisher="test" poster-portrait-src="test" publisher-logo-src="img.jpg" standalone>
<amp-story-page id="cover">
<amp-story-grid-layer template="vertical">
<h1>Hello World</h1>
<p>This is the cover page of this story.</p>
<amp-img data-hero src="https://amp.dev/static/samples/img/story_dog2.jpg" width="300" height="300" layout="responsive" alt="" />
</amp-story-grid-layer>
</amp-story-page>
</amp-story>
</body>
</html>
8 changes: 8 additions & 0 deletions packages/optimizer/spec/helpers/TransformerRunner.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
const validator = require('amphtml-validator');
const {basename, join} = require('path');
const {writeFileContents, getFileContents, getDirectories} = require('../helpers/Utils.js');

Expand Down Expand Up @@ -47,6 +48,7 @@ module.exports = function (testConfig) {
getDirectories(testConfig.testDir).forEach((testDir) => {
it(basename(testDir), async (done) => {
let params = TRANSFORMER_PARAMS;
const validatorInstance = await validator.getInstance();

// parse input and extract params
let input = getFileContents(join(testDir, 'input.html'));
Expand Down Expand Up @@ -86,6 +88,12 @@ module.exports = function (testConfig) {
} else {
expect(actualOutput).toBe(expectedOutput);
}
if (testConfig.validAmp) {
const result = validatorInstance.validateString(actualOutput);
if (result.status !== 'PASS') {
fail(`Validation errors:\n\n ${JSON.stringify(result.errors, null, 2)}`);
}
}
done();
});
});
Expand Down