Revisit aligned images with inline width larger than max getting displayed off-center

[westonruter]

As a user, I expect my images inside a post to be aligned correctly, even if they are larger than the max-width of the given viewport.

• AC1: Identify when and why the viewport breaks.
• AC2: A user who has a wider-than-viewport image expects their image to display in margin; using the plugin's sanitizer, we expect this to be fixed automatically.

---

The conversion of width to max-width was originally added in #495. This was removed recently in #1064 as it was not determined to be the best solution to the problem reported in #494 (“Aligned images with inline width larger than max get displayed off-center”)? In #610 it was suggested to be removed:

Notably, the current AMP spec does not […] specify that width should be replaced by max-width, thus these rules are no longer enforced.

It feels like it fixes a symptom but isn't the right solution to the underlying problem.

In #1064 (comment) @mehigh comments:

Instead of replacing the width by max-width, a solution used almost everywhere on the internets is to introduce a max-width: 100%; on the images. That way they would 'stay put' regardless on whether the width attribute is larger than the container or not. But there are edge cases, and nuances, as with any solutions.

So we need to (1) replicate the original issue and (2) determine what the best fix is.

[postphotos]

Hi @westonruter - I've added a user story and ACs here.

[hellofromtonya]

It feels like it fixes a symptom but isn't the right solution to the underlying problem.

I agree. Let's focus on investigating both the classic and Gutenberg editors to discover the root cause(s) of width and alignment issues.

AC1: Identify when and why the viewport breaks.

I ran an experiment on both the classic and Gutenberg editors to find out where the viewport breaks.
Here are the parameters I used:

1. Used the theme team's test data
2. Classic - used the "Markup: Image Alignment" post.
3. Gutenberg - converted the "Markup: Image Alignment" into separate paragraph and image blocks.
4. Used the Twenty Seventeen theme.

Results:

1. No errors found for wider images in the classic editor.
   • wider images rescaled to fit within the content
   • alignment was not an issue as the image filled the content width
2. Found 2 issues with the Gutenberg editor:
   • wider images extend out past the content in all viewports
   • center alignment is left aligned

What is the root cause for wider images in Gutenberg?
A class attribute of wp-block-image is applied to the <figure> that surrounds the <amp-img> element. That class has a width of fit-content.

Side-by-Side Comparison

Here's a side-by-side comparison with Gutenberg on the left and the Classic editor on the right:

View the example posts: Gutenberg post and Classic Editor post.

Box Models

HTML

This gist has the HTML for the 580px x 300px image in each editor.

Analyzing the Results

Why does the Gutenberg image extend outside of the container?

The <figure> has an extra class attribute called wp-block-image, i.e. the classic editor does not have this attribute. The CSS associated with this attribute sets the width to fit-content:

.wp-block-image {
    width: -webkit-fit-content;
    width: -moz-fit-content;
    width: fit-content;
}

Essentially, fit-content adjusts the width of its element to the width of its child nodes.

References: MDN | CSS Reference on Codrops with an excellent explanation from Sara Soueidan

Possible Solution

If we wish to contain the images within the content, then we need to change the width from fit-content to auto.

[hellofromtonya]

Possible Solution

Going deeper into the discussion above, Gutenberg adds the width: fit-content; as it's default width for the .wp-block-image attribute, as found here in the SCSS file. Notice that the width switches to min-content if it includes the class attribute of is-resized.

This means we can add a method into the AMP_Img_Sanitizer to decide if we should add an inline width. Here is the decision tree, for which each must be a true decision:

• $node->parentNode is an instance of DOMElement
• $node->parentNode->tagName is 'figure'.
• it has a 'wp-block-image' class attribute.
• it does not have a 'is-resized' class attribute.

If all of those decisions are true, then we can add an inline style width: auto;. That will override the default fit-content and achieve our goal of scaling wider images to fit within the content.

[westonruter]

This seems closely related to #1237, but maybe not. In that ticket, I think the problem is that amp-img does not support object-fit. In this ticket, is it related in that fit-content width is not compatible with amp-img like it is with img?

[hellofromtonya]

Upon further exploration, the AMP <amp-img> is overflowing the content area when its parent has a width of fit-content and the image is wider than the content area. However, the <img> in a non-AMP mode does not overflow with the same parent and image attribute values.

There is a systematic issue in the AMP rendering that appears to be causing the overflow.

PR #1281 fixes the rendering; however, it feels more like a bandaid than a solution. For now, this PR is put on hold until we can get an answer back from the AMPHTML project.

I've opened a ticket ampproject/amphtml/17084 to capture the issue and its behavior. This issue may be linked to issue #1237 and its open ticket ampproject/amphtml/17053.

[hellofromtonya]

PR #1281 fixes this issue. However, it's a potentially a bandaid. We'll keep this ticket open to track progress on the AMPHTML ticket ampproject/amphtml/17053. Once that ticket is resolved, we can then check for impact or regression issues to our ticket 1086.

[kienstra]

It looks like this isn't yet fixed in ampproject/amphtml#17053. So I'll probably move this issue to 'Ready For Merging' soon if that's alright, as it looks like it will be part of the RC release.

[kienstra]

Moving To 'Ready For Merging'

This will be part of the RC1 release, as it looks like the underlying issue is not yet fixed in ampproject/amphtml#17053.

[kienstra]

Removing From Milestone

If it's alright, I'm removing this from the v1.0 milestone. This issue is really to track ampproject/amphtml#17053, it doesn't require action unless that issue is fixed.

[westonruter]

This is being revisited in #2036. Please test.