Amp-wp: Image Not Rendering Properly on Mobile and Desktop

[MackenzieHartung]

Bug Description

The image entered during editing renders differently (erroneously) once published.

Expected Behaviour

Story should visually look the same in the editor and once published.

Steps to reproduce

1. Add image block to page
2. Preview/Publish
3. Image block displays incorrectly

Screenshots

Additional context

• WordPress version:
• Plugin version: 1.3-rc
• Gutenberg plugin version (if applicable):
• AMP plugin template mode:
• PHP version:
• OS:
• Browser: [e.g. chrome, safari]
• Device: [e.g. iPhone6]

---

_Do not alter or remove anything below. The following sections will be managed by moderators only._

Acceptance criteria

• AC1: When creating stories in the editor, image within image blocks should display the same in the preview/publish view.

Implementation brief

• The issue of 0% displaying as 5% comes from PHP where empty gives positive value for 0, this causes using the default 5 as the value:
  On 0% being displayed as 5%: yep, 5 is indeed the default value for positionLeft. Looks like the issue comes from PHP where 0 is considered empty :
  https://github.com/ampproject/amp-wp/blob/5799032d4cc9882ceac94966f67c318ebb60eb68/includes/class-amp-story-post-type.php#L1534

We should replace using empty by something more reliable, perhaps with isset or strict equals for 0.

• The weird displaying of images due to display:table can be fixed by overriding the value in the plugin's amp-stories-frontend.css file for .wp-block-image.is-resized and other .wp-block-image.* values. See /gutenberg/packages/block-library/src/image/style.scss to check where the display:table is set. The value should be display: block instead.

QA testing instructions

Test 1

1. Add image block to the page
2. Preview/Publish
3. Verify that the block matches the view in the editor and in the FE (unlike on the screenshots above)

Test 2

1. Add an image block to the Page
2. Place the image exactly on the left border of the Page
3. Verify that it displays on the border of the Page in the FE as well.

Demo

• https://youtu.be/P_WEsErfmUk

Changelog entry

• Improved rendering of images on the frontend to be more accurate

[spacedmonkey]

I believe this issue is related to the calculations to get font size based on text box size being different from the stories editor compared the AMP stories front end. I believe it is going to hard to each calculations to exactly match.

[barklund]

The text is placed exactly like in the editor. It's the image that's completely off.

These are the screenshots next to each other with some guides on top:

[jauyong]

@barklund I've updated the ticket to indicate the issue is regarding the image, not text.

[spacedmonkey]

Possibly related https://github.com/ampproject/amp-wp/issues/3498

[barklund]

Problem 1

If you place an item directly on the left edge, exactly at left: 0%, in the resulting story it is placed at left: 5%.

I guess it's a default somewhere, but 0 here is not a lack of coordinate, it's specifically 0 and should be respected.

Problem 2

The display model for the <figure> containing a placed image is set to table, which can render it very weirdly, often rendering it at a wrong height. Simply disabling this line of CSS and then re-enabling it, fixes the image layout. Setting it to display:block of course also fixes it.

Why is it set to table and where does it come from? I've tracked it to this inline style in the HTML (line 3 in the screenshot), but I can't find any amp-custom.css anywhere, so I have no idea who has defined this:

It's present both in my local install and in the Pantheon-site, so it must be defined in the plugin and not as some custom setting (it could also be the 2019 theme, but that seems less likely). I have no idea where it's defined nor why.

Example for both

| In Editor |
| -- |
|  |
| _An image has been placed in the upper left corner of the page at 0,0 taking up (as well as possible) exactly 25% of the entire page (link to Pantheonsite)._ |

| In Story |
| - |
| |
| _Here, the image is clearly not placed in the corner, and the height is way off (the width is correct)._ |

Conclusion

I really don't know where to fix either issue.

Thoughts for fixing either, @miina, @swissspidy, @spacedmonkey?

[swissspidy]

IIRC the table layout is defined by the Gutenberg block library CSS. We could override that.

amp-custom is an artificial name as the AMP plugin gathers all CSS used on a page and puts it into that inline style tag.

[miina]

Thanks for looking into this issue!

On 0% being displayed as 5%: yep, 5 is indeed the default value for positionLeft. Looks like the issue comes from PHP where 0 is considered empty :
https://github.com/ampproject/amp-wp/blob/5799032d4cc9882ceac94966f67c318ebb60eb68/includes/class-amp-story-post-type.php#L1534

On the CSS:

• AMP doesn't allow external stylesheets on AMP Pages and thus the plugin processes all the stylesheets applied to the page, removes the disallowed and unused CSS rules, and then puts it all together to display in allowed format (<style amp-custom>).
• Looks like the display:table comes from /gutenberg/packages/block-library/src/image/style.scss file, if I understand correctly. We could override this in our plugin.

[swissspidy]

Just to clarify, it sounds like overriding display:table and changing the empty check to something else might be all that's needed?

[miina]

Just to clarify, it sounds like overriding display:table and changing the empty check to something else might be all that's needed?

Yes, looks like it.

[barklund]

@miina, will you do the IB? You know these parts of the project a lot better than me. You're also more or less there with the above comment 💪.

[miina]

Sure!

[miina]

Moved to IBR!

[spacedmonkey]

Approved IBR.

[barklund]

Looks good to me. Let's get this executed.

However, do remember to do a git blame on that display:table to check with whomever did the original code to ensure, that we don't screw up something else.

[miina]

Note that display: table comes from the Gutenberg (upstream) plugin and not from the AMP Plugin. AMP Plugin combines _all the_ CSS, from other plugins as well, to make it valid for an AMP Page, that's why it shows up in the amp-custom.css (the dynamically created CSS).

[spacedmonkey]

@miina This seems like a S to me. As it seems you have basically done the work in your head / discovery.

[miina]

Sounds good, moving to Todo!

[csossi]

Verified in QA