🐛 Restrict display:block/position:relative styles to implicit responsive layout

[westonruter]

In #27083 the disable-inline-width attribute was introduced to allow “specifying the sizes on elements without getting the sizing behavior of sizes in AMP (which adds a width based on which media query matches).” This addressed #21736.

In the WordPress AMP plugin we wrestled a lot with AMP's sizes behavior (#17053), leading us ultimately to remove the sizes attribute altogether to avoid it (ampproject/amp-wp#2036). This was not ideal because even though WordPress generates images with sizes and srcset, for the sake of AMP we had to strip the sizes out.

When updating the validator spec in the plugin, I discovered the new disable-inline-width attribute and confirmed with @caroqliu that it would prevents AMP's default sizes behavior.

So we proceeded to eliminate the removal of the sizes attribute in favor of adding the disable-inline-width attribute when converting img to amp-img. It all seemed to work perfectly except for one particular case, where an image is added inline within a paragraph.

In the non-AMP version and the AMP version when sizes is removed, the inline image is rendered like so:

However, when sizes is retained with disable-inline-width added, the result is the image appearing on its own line:

This is due to this CSS rule in ampshared.css:

amphtml/css/ampshared.css

Lines 59 to 65 in 97099ce

	.i-amphtml-layout-responsive,
	[layout=responsive][width][height]:not(.i-amphtml-layout-responsive),
	[width][height][sizes]:not(.i-amphtml-layout-responsive)
	{
	display: block;
	position: relative;
	}

It appears that the last selector in this rule ([width][height][sizes]:not(.i-amphtml-layout-responsive)) was not updated to account for the presence of the disable-inline-width attribute:

- [width][height][sizes]:not(.i-amphtml-layout-responsive)
+ [width][height][sizes]:not([disable-inline-width]):not(.i-amphtml-layout-responsive)

The result is the element unexpectedly getting a display:block style rather than the expected display:inline-block.

Is there any need for elements with the disable-inline-width attribute to have these styles applied? If not, this PR excludes such elements from getting the styles unnecessarily.

[jridgewell]

Can you link me to a page that displays this?

It appears that the last selector in this rule was not updated to account for the presence of the disable-inline-width attribute:

Note that the second and third selectors are meant to be a pair, so they need to be updated together.

[westonruter]

@jridgewell here you go: https://amp-img-sizes-display-inline-width-test.glitch.me/

[jridgewell]

Ok, I think this is actually due to intrinsic not properly overriding the implicit responsive styles. We have a <amp-img layout=intrinsic ...>, but it's applying layout=responsive styles to it because we forgot to exclude intrinsic from the implicit detection. What we actually need is:

-[width][height][sizes]:not(.i-amphtml-layout-responsive)
+[width][height][sizes]:not([layout=intrinsic]):not(.i-amphtml-layout-responsive)

Or, to specify the implicit responsive only applies if there is no layout:

-[width][height][sizes]:not(.i-amphtml-layout-responsive)
+[width][height][sizes]:not([layout])

---

Unless, you feel this should also apply to layout=responsive elements with disable-inline-width?

[westonruter]

I'm not sure what is preferable. In a WordPress context, we use intrinsic as the default layout when converting img to amp-img, so it's really the only place where we've noticed this issue. The alternative selectors you suggested also fix the issue, but I defer to your judgement about what should be done.

[jridgewell]

I think we should update it to [width][height][sizes]:not([layout]):not(.i-amphtml-layout-responsive). The selector is meant to target implicit responsive only, and implicit means no layout attr. And it should no longer apply when the JS has initialized and added the .i-amphtml-layout-responsive (to prevent it from affecting precedence).

We can open a new PR if we want to add inline-block to responsive images.

[westonruter]

I think we should update it to [width][height][sizes]:not([layout]):not(.i-amphtml-layout-responsive).

Applied in 3decb89.