Amphtml: Validator counts SSR-inserted style attributes against style amp-custom max bytes of 75000

[westonruter]

Consider an AMP document that includes an style[amp-custom] that is maxed out at 75,000 bytes (or close to that limit). If this document also includes any AMP elements which have a layout such as fixed, then server-side rendering for transformed/optimized AMP will a style attribute to specify the width and height (which eliminates the need for the AMP boilerplate CSS). The inclusion of this style attribute will then cause the total amp-custom size to go above the 75,000 limit.

This issue came up in the WordPress AMP plugin: https://github.com/ampproject/amp-wp/issues/5517.

Example unoptimized AMP document with 75,000 bytes in style[amp-custom]: https://amp-custom-style-ssr-overage-example.glitch.me/unoptimized.html

This same AMP document transformed with the Node.js AMP Optimizer: https://amp-custom-style-ssr-overage-example.glitch.me/optimized.html

Note that the latter is invalid where as the former is not: https://validator.amp.dev/#url=https%3A%2F%2Famp-custom-style-ssr-overage-example.glitch.me%2Foptimized.html

The author stylesheet specified in tag 'style amp-custom' and the combined inline styles is too large - document contains 75025 bytes whereas the limit is 75000 bytes.

The Go AMP Optimizer also causes this problem as can be seen by accessing the unoptimized document on the AMP Cache: https://amp--custom--style--ssr--overage--example-glitch-me.cdn.ampproject.org/c/s/amp-custom-style-ssr-overage-example.glitch.me/unoptimized.html

And attempting to access the pre-optimized AMP document on the AMP Cache is rejected due to it not being valid: https://amp--custom--style--ssr--overage--example-glitch-me.cdn.ampproject.org/c/s/amp-custom-style-ssr-overage-example.glitch.me/optimized.html

Proposal

I propose that the validator skip counting the bytes represented by the width and height properties of style attributes which occur on elements with the i-amphtml-layout attribute.

Either this, or style attributes should be exempted altogether per #17269.

This also relates to #27468 which is for style rules extracted during SSR for media and heights attributes.

[westonruter]

cc @ampproject/wg-caching

[honeybadgerdontcare]

This is also tracked internally at b/127517621

[westonruter]

Possible implementation for Node.js:

diff --git a/validator/js/engine/validator.js b/validator/js/engine/validator.js
index 3a3941e07..9a02b1172 100644
--- a/validator/js/engine/validator.js
+++ b/validator/js/engine/validator.js
@@ -4876,12 +4876,20 @@ function validateScriptSrcAttr(srcAttr, tagSpec, context, result) {
  * @param {!generated.TagSpec} tagSpec
  * @param {string} attrName
  * @param {string} attrValue
+ * @param {bool} hasTransformedLayout
  * @param {!ValidateTagResult} result
  */
 function validateAttrCss(
-    parsedAttrSpec, context, tagSpec, attrName, attrValue, result) {
+    parsedAttrSpec, context, tagSpec, attrName, attrValue, hasTransformedLayout, result) {
+  let normalizedAttrValue = attrValue;
+
+  // Omit bytes for inline width/height styles for elements have been transformed.
+  if (hasTransformedLayout) {
+    normalizedAttrValue = normalizedAttrValue.replace(/(width|height):.+?;/g, '')
+  }
+
   /** @type {number} */
-  const attrByteLen = htmlparser.byteLength(attrValue);
+  const attrByteLen = htmlparser.byteLength(normalizedAttrValue);
   // Track the number of CSS bytes. If this tagspec is selected as the best
   // match, this count will be added to the overall document inline style byte
   // count for determining if that byte count has been exceeded.
@@ -5302,7 +5310,7 @@ function validateAttributes(
     }
     if (attrSpec.valueDocCss || attrSpec.valueDocSvgCss) {
       validateAttrCss(
-          parsedAttrSpec, context, spec, attr.name, attr.value, result);
+          parsedAttrSpec, context, spec, attr.name, attr.value, 'i-amphtml-layout' in attrsByName, result);
     } else if (attrSpec.cssDeclaration.length > 0) {
       validateAttrDeclaration(
           parsedAttrSpec, context, getTagDescriptiveName(spec), attr.name,

[honeybadgerdontcare]

I'd prefer the solution mentioned in the link I shared: While validating server-side rendering changes, count how many bytes are being added as inline styles, then subtract that when checking for the byte limit.

Isn't the WordPress plugin already arbitrarily reducing the number of bytes in style[amp-custom]? Is it possible as a stop-gap measure to have it account for this and reduce it slightly more?

[westonruter]

Isn't the WordPress plugin already arbitrarily reducing the number of bytes in style[amp-custom]? Is it possible as a stop-gap measure to have it account for this and reduce it slightly more?

The issue is that the plugin is sanitizing and validating style[amp-custom] at an intermediary state on a pre-optimized document. The AMP plugin only works with unoptimized AMP documents as input. It then passes the validated unoptimized AMP document over to the Optimizer to do server-side rendering, so the style attributes added by the Optimizer don't exist at the time of sanitization.

[schlessera]

The following attributes should also be allowed without size penalty in inline styles: object-position & object-fit.

These are currently being copied as regular attributes during SSR operations by all Optimizer implementations, but then the AMP Runtime moves them to inline styles, causing a layout shift. To avoid that layout shift, the SSR operation should immediately turn them into an inline style.

See https://github.com/ampproject/amp-toolbox-php/issues/6

[honeybadgerdontcare]

These are currently being copied as regular attributes during SSR operations by all Optimizer implementations

This is not the case for all SSR implementations though. Let's start with those attributes inserted by SSR implementations. We can look at additional feature requests post that.

[honeybadgerdontcare]

I'm closing this in favor of #32004. Please provide feedback/comments there.