✨ Validator support for transformed intrinsic layout

[sebastianbenz]

Intrinsic layout SSR adds the following sizer element:

<amp-img height="100" width="300" layout="intrinsic" class="i-amphtml-layout-intrinsic i-amphtml-layout-size-defined" i-amphtml-layout="intrinsic">
  <i-amphtml-sizer class="i-amphtml-sizer">
    <img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;charset=utf-8,<svg height=&quot;100&quot; width=&quot;300&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>">
  </i-amphtml-sizer>
</amp-img>

This commit extends the i-amphtml-sizer validation rules to
alternatively support the intrinsic sizer element. The SVG sizer image
is validated via a regex checking if the src value is an SVG.

[Gregable]

can this just be value:?

Note that in validation rules, value_regex always means full match and blacklisted_value_regex is partial match. This is a bit confusing, but it is in place to avoid mistakes in the wrong direction (looser validation rules than expected).

If you intended partial match, use ".*i-amphtml-intrinsic-sizer.*" and also ignore my dispatch key comment.

[sebastianbenz]

First I had it as value, but that caused the test to fail. I couldn't work out why. I assumed that was specific to how class is handled:

    |      <i-amphtml-sizer class="i-amphtml-sizer"><img alt="" aria-hidden="true" class="i-amphtml-intrinsic-sizer" role="presentation" src="data:image/svg+xml;charset=utf-8,<svg height=&quot;100&quot; width=&quot;300&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>"></i-amphtml-sizer>
    >>                                              ^~~~~~~~~
    transformed_feature_tests/server_side_rendering.html:40:45 The mandatory attribute 'class' is missing in tag 'IMG-I-AMPHTML-INTRINSIC-SIZER'. [DISALLOWED_HTML]

[Gregable]

Thats odd. Is the example you give the result for that test? Did you verify the spelling?

[sebastianbenz]

Finally had some time to look into this again. I've addressed the comments. The current version:

• passes the tests
• accepts the intrinsic layout sizer

However, theoretically there are still a few validation rules possible:

• ensure that sizer matches layout, e.g. currently it'd be valid for the responsive sizer to have an img child.
• ensure that the class attribute is set for the intrinsic sizer: <i-amphtml-sizer class="i-amphtml-sizer">

I'd need some guidance though on how to best implement these in the validator (in particular how to figure out from a child ndoe whether one of the parent nodes has a specific attribute value set).

[sebastianbenz]

I've split i-amphtml-sizer into two tag specs. Now, sizer attributes area correctly verified. I couldn't figure out though how to set I-AMPHTML-SIZER-INTRINSIC as the mandatory parent.

PTAL

[amp-owners-bot]

Hey @ampproject/wg-caching, these files were changed:

• validator/validator-main.protoascii

[Gregable]

The amp-owners-bot popped this back to my visibility. What's the latest status on this? Do you need additional review?

[sebastianbenz]

I've updated the PR to add the missing check for the intrinsic parent (now that b/139869098 has landed). PTAL

[Gregable]

Looks good to me. @honeybadgerdontcare did you have any other requirements? When ready, please approve.

[honeybadgerdontcare]

lgtm after nit addressed