Make WordPress Core

Opened 9 months ago

Closed 5 months ago

Last modified 2 months ago

#59352 closed defect (bug) (fixed)

Inline images inserted in the block editor can erroneously get fetchpriority=high

Reported by: westonruter's profile westonruter Owned by: flixos90's profile flixos90
Milestone: 6.5 Priority: normal
Severity: normal Version: 6.3
Component: Media Keywords: has-patch has-unit-tests commit
Focuses: performance Cc:

Description (last modified by westonruter)

I found that inserting an inline image in the block editor can result in it erroneously getting fetchpriority=high even though it is normally rendered as a small image. When inserting an inline image, the default size (width) is 150 pixels. Nevertheless, the originally uploaded image is actually chosen for the src (along with its width and height) and the user-supplied size is added as an inline width CSS style. Here is the rendered markup for an inline image appearing as the first image in the content:

<img
  decoding="async"
  fetchpriority="high"
  width="534"
  height="534"
  class="wp-image-143"
  style="width: 30px"
  src="http://example.org/wp-content/uploads/2023/09/u1f629_u1f973.png"
  alt="Anguished partygoer emoji"
  srcset="
    http://example.org/wp-content/uploads/2023/09/u1f629_u1f973.png         534w,
    http://example.org/wp-content/uploads/2023/09/u1f629_u1f973-300x300.png 300w,
    http://example.org/wp-content/uploads/2023/09/u1f629_u1f973-150x150.png 150w
  "
  sizes="(max-width: 534px) 100vw, 534px"
/>

The fetchpriority=high attribute is added to this image even when it is immediately followed by an image block with a large size.

To address this, wp_get_loading_optimization_attributes() could take into account $attr['style'] and parse out any width property which would override $attr['width']. This should address inline images from the block editor, but note it would not account for images that are resized by CSS style rules.

Aside: The sizes attribute here, at least in Twenty Twenty-Three, is causing the full size image to be rendered even though the thumbnail size is included in the srcset. It seems like whatever the user supplied as the size of the image (e.g. 30px here) should be mirrored between the inline style attribute's width and the sizes attribute. So here it could be sizes="30px".

Attachments (3)

u1f629_u1f973.png (20.8 KB) - added by westonruter 9 months ago.
Anguished partygoer emoji, courtesy of [emoji kitchen] on Google.com
inline-image-menu-item.png (281.2 KB) - added by westonruter 9 months ago.
Selecting an inline image from paragraph block in block editor
inline-image-inserted.png (281.0 KB) - added by westonruter 9 months ago.
After inserting an inline image with initial size set to 150px

Download all attachments as: .zip

Change History (17)

@westonruter
9 months ago

Anguished partygoer emoji, courtesy of [emoji kitchen] on Google.com

@westonruter
9 months ago

Selecting an inline image from paragraph block in block editor

@westonruter
9 months ago

After inserting an inline image with initial size set to 150px

#1 follow-up: @joemcgill
9 months ago

It seems to me like the real bug here is that an inline style is being added to control the width of this image instead of updating the width attribute. This also means that WP is incorrectly creating a sizes attribute based on the width attribute, which is being overridden by the inline CSS.

I'd suggest that we look into fixing the markup generated by the inline image component, rather than adjusting optimizations to take these inline styles into account as a first step.

#2 @westonruter
9 months ago

  • Description modified (diff)

#3 in reply to: ↑ 1 @westonruter
9 months ago

Replying to joemcgill:

It seems to me like the real bug here is that an inline style is being added to control the width of this image instead of updating the width attribute. This also means that WP is incorrectly creating a sizes attribute based on the width attribute, which is being overridden by the inline CSS.

Good point. I made a POC plugin to fix up the issue using HTML Tag Processor: https://gist.github.com/westonruter/f37f747a57643462b7efe110dd61d91b

To fix this in core, it seems like this would need to be a special case in wp_img_tag_add_width_and_height_attr(), yeah?

#4 @flixos90
8 months ago

  • Milestone changed from Future Release to 6.5
  • Owner set to flixos90
  • Status changed from new to assigned

Tentatively moving this to the 6.5 milestone for further investigation.

#5 @flixos90
6 months ago

I agree that the root of the problem is in the inline image component in Gutenberg. That said, it could be fixed either way. The existing wp_img_tag_add_width_and_height_attr() function was originally introduced as a fallback to add missing dimension attributes already partly because Gutenberg wasn't always adding those attributes, so I think adding a special condition for this use case could work.

Doing it in that function is also more reliable as it will address the problem for existing content, while doing it in Gutenberg would likely only fix it for new content created in the future.

This ticket was mentioned in PR #5767 on WordPress/wordpress-develop by @flixos90.


6 months ago
#6

  • Keywords has-patch added; needs-patch removed

See https://core.trac.wordpress.org/ticket/59352#comment:3: This adds an extra condition to wp_img_tag_add_width_and_height_attr() which ensures the dimension attribute respects an enforced width via style attribute.

Trac ticket: https://core.trac.wordpress.org/ticket/59352

@westonruter commented on PR #5767:


6 months ago
#7

I just tested this PR with Playground and it worked. Wow, that is a nice way to test.

Editor:

https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/134745/0422665a-7db2-4aa8-a68d-dc226ff1217e

Post content:

This is inline: [[Image(https://i0.wp.com/playground.wordpress.net/scope:0.6233595411696027/wp-content/uploads/2023/12/anguished-partygoer.png)]]

Frontend HTML:

This is inline: [[Image(https://i0.wp.com/playground.wordpress.net/scope:0.6233595411696027/wp-content/uploads/2023/12/anguished-partygoer.png)]]

The width and height attributes are added.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


6 months ago

@flixos90 commented on PR #5767:


5 months ago
#9

@westonruter @joemcgill I just added test coverage (see updated PR description). Given you already approved, I'm planning to commit this tomorrow, but please take a look at the tests if you can.

#10 @flixos90
5 months ago

  • Keywords has-unit-tests commit added

This ticket was mentioned in Slack in #core-performance by flixos90. View the logs.


5 months ago

#12 @flixos90
5 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 57294:

Media: Consider inline image CSS width to backfill width and height attributes.

Prior to this changeset, WordPress core would use the original image size, which in the particular case of inline images would be severely off, as they are usually very small. This could lead to incorrect application of fetchpriority="high" and other performance optimizations.

Props westonruter, flixos90, joemcgill, mukesh27.
Fixes #59352.

#14 @audrasjb
2 months ago

In 58033:

Media: Prevent division by zero in wp_img_tag_add_width_and_height_attr().

This changesets adds a check for $size_array values to prevent a potential division by zero.
Follow-up to [57294].

Props jdekhtiar.
Fixes #61054.
See #59352.

Note: See TracTickets for help on using tickets.