Elements with decimal dimensions cause AMP validation errors in i-amphtml-sizer for Optimized/Transformed AMP

[westonruter]

Originally reported in ampproject/amp-wp#4493.

Given input HTML like so:

<amp-img src="https://blog.amp.dev/wp-content/uploads/2020/03/AMP_camp_Blog.png" alt="" height="938" width="1333.8226950355" layout="intrinsic"></amp-img>

Take note of the decimal value in the width. This is valid AMP.

When the Optimizer is enabled (in both the Node.js and PHP version), this gets optimized as:

<amp-img src="https://blog.amp.dev/wp-content/uploads/2020/03/AMP_camp_Blog.png" alt="" height="938" width="1333.8226950355" 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;938&quot; width=&quot;1333.8226950355&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>">
    </i-amphtml-sizer>
</amp-img>

Which is not valid AMP. A validation error is raised:

The attribute 'src' in tag 'IMG-I-AMPHTML-INTRINSIC-SIZER' is set to the invalid value 'data:image/svg+xml;charset=utf-8,<svg height="938" width="1333.8226950355" xmlns="http://www.w3.org/2000/svg" version="1.1"/>'.

When the AMP optimizer is disabled, the AMP runtime is generating constructing the DOM as follows:

<amp-img src="https://blog.amp.dev/wp-content/uploads/2020/03/AMP_camp_Blog.png" alt="" height="938" width="1333.8226950355" layout="intrinsic" class="i-amphtml-element i-amphtml-layout-intrinsic i-amphtml-layout-size-defined i-amphtml-layout" i-amphtml-layout="intrinsic" style="--loader-delay-offset:119ms !important;">
	<i-amphtml-sizer class="i-amphtml-sizer">
		<img alt="" role="presentation" aria-hidden="true" class="i-amphtml-intrinsic-sizer" src="data:image/svg+xml;charset=utf-8,<svg height=&quot;938px&quot; width=&quot;1333.8226950355px&quot; xmlns=&quot;http://www.w3.org/2000/svg&quot; version=&quot;1.1&quot;/>">
	</i-amphtml-sizer>
	<img decoding="async" alt="" src="https://blog.amp.dev/wp-content/uploads/2020/03/AMP_camp_Blog.png" class="i-amphtml-fill-content i-amphtml-replaced-content">
</amp-img>

Note that the decimal width is used here as well. This indicates that the validator spec is at fault here, namely the IMG-I-AMPHTML-INTRINSIC-SIZER spec which defines the src attribute to have a value_regex as follows:

amphtml/validator/validator-main.protoascii

Line 4879 in 2984894

value_regex: "data:image\\/svg\\+xml;charset=utf-8,<svg height=\"\\d+\" width=\"\\d+\" xmlns=\"http:\\/\\/www\\.w3\\.org\\/2000\\/svg\" version=\"1\\.1\"\\/>|data:image\\/svg\\+xml;charset=utf-8,<svg height='100' width='300' xmlns='http:\\/\\/www\\.w3\\.org\\/2000\\/svg' version='1\\.1'\\/>"

This needs to be changed to allow for decimal values, for example:

"data:image\\/svg\\+xml;charset=utf-8,<svg height=\"\\d+(\.\\d+)?\" width=\"\\d+(\.\\d+)?\" xmlns=\"http:\\/\\/www\\.w3\\.org\\/2000\\/svg\" version=\"1\\.1\"\\/>|data:image\\/svg\\+xml;charset=utf-8,<svg height='100' width='300' xmlns='http:\\/\\/www\\.w3\\.org\\/2000\\/svg' version='1\\.1'\\/>"

[westonruter]

cc @ampproject/wg-caching @sebastianbenz

[Gregable]

According to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img:

width: The intrinsic width of the image in pixels. Must be an integer without a unit.

I think this would indicate that the bug is in accepting non-integer widths for amp-img, if any, but I don't think we could reverse that decision at this point.

[westonruter]

So this is squarely an Optimizer bug?

[Gregable]

My argument is that there is also a bug in that the validator accepts decimal widths for amp-img, but that this is probably not a bug we can fix at this point due to the risk of de-validating documents.

One could also interpret the spec of width in amp-img as different than the spec of width in img, but I'm not sure if there is any reason to do so.

I suppose we could relax this in the validator anyway if you think that would be easier than the optimizer. I'm not sure if it would hurt anything.

[sebastianbenz]

Agree with Greg, it probably makes sense to relax the validator rules for the intrinsic sizer (at least the behavior is then consistently wrong). That's not something that should be fixed on optimizer side.

[westonruter]

Decimal values for width and height are actually rendered with sub-pixel resolution in Chrome, Safari, and Firefox: https://codepen.io/westonruter/pen/JjdVoqN

If we wanted to only output valid HTML5, then an alternative would be to use CSS to specify the sub-pixel widths:

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

[westonruter]

So should this be reopened? Let me know the direction which will be taken. We'll probably do a hotfix to the AMP plugin to force integer values to be used while waiting for a potential validator update.

[Gregable]

Sure. Reopening. Would you be willing to make the PR for the validator change?

[westonruter]

I can but @sebastianbenz would probably be able to get to it first.