Amphtml: [Story Player] AMP validator failures for <amp-story-player> for responsive layout

Created on 26 Sep 2020  路  12Comments  路  Source: ampproject/amphtml

Given this amp-story-player example, which is taken from the docs but replacing fixed with responsive:

  <amp-story-player layout="responsive" width="360" height="600">
    <a
       href="https://preview.amp.dev/documentation/examples/introduction/stories_in_amp/"
       style="--story-player-poster: url('https://amp.dev/static/samples/img/story_dog2_portrait.jpg')"
    >
      Stories in AMP - Hello World
    </a>
  </amp-story-player>

If an amp-story-player is transformed via the Node.js AMP Optimizer, the result is this:

<amp-story-player layout="responsive" width="360" height="600" class="i-amphtml-layout-responsive i-amphtml-layout-size-defined" i-amphtml-layout="responsive">
  <i-amphtml-sizer style="display:block;padding-top:166.6667%;"></i-amphtml-sizer>
  <a href="https://preview.amp.dev/documentation/examples/introduction/stories_in_amp/" style="--story-player-poster: url('https://amp.dev/static/samples/img/story_dog2_portrait.jpg')"> Stories in AMP - Hello World </a>
</amp-story-player>

This results in another validation error:

Tag 'i-amphtml-sizer' is disallowed as child of tag 'amp-story-player'. Child tag must be one of ['a'].

See example in AMP Playground.

Bug caching stories

All 12 comments

cc @ampproject/wg-caching @pbakaus @swissspidy

cc @ampproject/wg-stories

This seems like a concrete case of #24262.

Agree with @newmuis that this is a dupe of #24262

@westonruter you may also want to file a documentation bug against amp.dev

@Gregable @newmuis what about the second part of the issue regarding i-amphtml-sizer?

Agree with @newmuis that this is a dupe of #24262

The second part is a dupe, but the first is not. I'll update the description to reduce the scope.

Reopening until the i-amphtml-sizer part is addressed.

I think we need to add I-AMPHTML-SIZER as an allowed descendant of the amp-story-player. Example:

https://github.com/ampproject/amphtml/blob/303ed4a51e54bdbfd8e06a41650b0c48544fdd67/extensions/amp-story/validator-amp-story.protoascii#L1035

@Enriqe The update in #30434 doesn't seem to have fixed the issue. See https://github.com/google/web-stories-wp/pull/5958#issuecomment-762278307.

Specifically, it accounts for the responsive layout but not the intrinsic one. In the responsive layout, the sizer looks like:

<i-amphtml-sizer style="display:block;padding-top:166.6667%;"></i-amphtml-sizer>

Whereas in the intrinsic layout, the sizer looks like:

<i-amphtml-sizer class="i-amphtml-sizer">
    <img alt aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;base64,PHN2ZyBoZWlnaHQ9IjYwMCIgd2lkdGg9IjM2MCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIiB2ZXJzaW9uPSIxLjEiLz4=">
</i-amphtml-sizer>

I'll open a new issue specifically for the intrinsic layout.

See #32040

Was this page helpful?
0 / 5 - 0 ratings