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.

Sign up for GitHub

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

Jump to bottom

AMP Optimizer: Why occur the error related to PreloadHeroImage? #894

Closed
sudokzt opened this issue Aug 15, 2020 · 4 comments · Fixed by #896
Closed

AMP Optimizer: Why occur the error related to PreloadHeroImage? #894

sudokzt opened this issue Aug 15, 2020 · 4 comments · Fixed by #896

Comments

@sudokzt
Copy link

sudokzt commented Aug 15, 2020 •
edited
Loading

I use amp-optimizer with NextJS. The error occur in the page that used <amp-story>
Before I update @ampproject/toolbox-optimizer to 2.6.0, the error is not occured.
Maybe this error related to PreloadHeroImage. Because if I turn off the PreloadHeroImage, this is fine.

Could you tell me why I am getting this error?

Reproduction

Steps to reproduce the issue

  1. prepare html(story-sample.html) that used amp-story
<!DOCTYPE html>
<html amp>
  <head>
    <title>Story page</title>
    <meta name="description" content="story page" />
    <meta charset="UTF-8"/>
    <meta name="viewport" content="width=device-width,initial-scale=1.0,minimum-scale=1.0"/>
    <link rel="canonical" href="https://amp.dev/">
    <script async src="https://cdn.ampproject.org/v0.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
      custom-element="amp-story"
      src="https://cdn.ampproject.org/v0/amp-story-1.0.js"
    ></script>
  </head>
  <body>
    <amp-story
      standalone=""
      title="title"
      publisher="sample"
      publisher-logo-src="https://amp.dev/static/samples/img/story_dog2.jpg"
      poster-portrait-src="https://amp.dev/static/samples/img/story_dog2.jpg"
    >
      <amp-story-page id="1">
        <amp-story-grid-layer template="fill">
          <amp-img
            src="https://amp.dev/static/samples/img/story_dog2.jpg"
            width="1"
            height="1"
            layout="responsive"
            alt=""
          />
        </amp-story-grid-layer>
      </amp-story-page>
    </amp-story>
  </body>
</html>
  1. run amp-optimizer to story-sample.html in the CLI

@ampproject/toolbox-cli optimize story-sample.html > story-optimized.html

  1. run amp-validator to story-optimized.html

amphtml-validator story-optimized.html

What's the expected result?

  • story-sample.html: PASS

What's the actual result?

  • Error The parent tag of tag 'img' is 'amp-img', but it can only be 'i-amphtml-sizer-intrinsic'.

use versions

next: "9.5.2", (but, probably this error is not directly related to NextJS)
@ampproject/toolbox-core: 2.6.0
@ampproject/toolbox-optimizer: 2.6.0
@sudokzt sudokzt mentioned this issue Aug 15, 2020
Closed
@sebastianbenz
Copy link
Collaborator

Can you please provide a code sample.

@sudokzt
Copy link
Author

sudokzt commented Aug 17, 2020

@sebastianbenz
Okay, sorry. revised this issue description.

By the way, if don't use amp-story, then in short, if use only amp-img, it will work.

<!DOCTYPE html>
<html amp>
  <head>
    <title>Sample Page</title>
    <meta name="description" content="sample page" />
    <meta charset="UTF-8"/>
    <meta name="viewport" content="width=device-width,initial-scale=1.0,minimum-scale=1.0"/>
    <link rel="canonical" href="https://amp.dev/">
    <script async src="https://cdn.ampproject.org/v0.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>
  </head>
  <body>
    <amp-img
      src="https://amp.dev/static/samples/img/story_dog2.jpg"
      width="1"
      height="1"
      layout="responsive"
      alt=""
    />
  </body>
</html>

then, run @ampproject/toolbox-cli optimize sample.html > sample-optimized.html and amphtml-validtor sample-optimized.html will output PASS

@sebastianbenz
Copy link
Collaborator

Thanks - that helps! Yeah, looks like the validator doesn't support this yet for stories.

@sebastianbenz
Copy link
Collaborator

Filed ampproject/amphtml#29850

sebastianbenz added a commit that referenced this issue Aug 17, 2020
@sebastianbenz
Don't ssr amp-img in stories
c1cfcb9 
Fixes #894
@sebastianbenz sebastianbenz mentioned this issue Aug 17, 2020
Merged
sebastianbenz added a commit that referenced this issue Aug 17, 2020
@sebastianbenz
Don't ssr amp-img in stories (#896)
3429af9 
Fixes #894
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't ssr amp-img in stories ampproject/amp-toolbox
2 participants
@sebastianbenz @sudokzt