Make WordPress Core

Opened 6 years ago

Last modified 5 years ago

#44801 new defect (bug)

`image_constrain_size_for_editor()` forcing the `$content_width` messes up dimensions

Reported by: flixos90's profile flixos90 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: needs-unit-tests has-patch
Focuses: Cc:

Description

The image_constrain_size_for_editor() currently looks at the $content_width global that themes may define. For medium_large, large and any custom image sizes, the actual image size's width will be evaluated against the $content_width, and the minimum will be used.

This results in the following (example):

  • Let's say $content_width is 640.
  • Let's say your large image size constraints are 1280x1280.
  • Let's say your image in large size is 1280x960.
  • Instead of using the image size width (1280), it will use the $content_width since it's smaller (640), resulting in 640x480 being used for the dimension of the image in large size.

This makes sense for the default behavior, but that computed size is used in many different areas, including wp_prepare_attachment_for_js(), where the sizes property then would show something like this for the large size (continuing the above example):

`js
{ width: 640, height: 480, url: '.../my-image-1280x960.jpg', orientation: 'landscape' }
`

Regarding what I want to accomplish with the custom image sizes, this results in unexpected behavior. For example in my case, I was looking into dynamically generating media queries for a background image, using the sizes. I was expecting for my code to generate @media (max-width: 1280px) ... for the image, but it would generate @media (max-width: 640px) ... due to the changed size.

As I already said, the $content_width global makes sense to take into account by default. However, it is currently hardcoded into the function. I think it should be moved into a filter (we already have editor_max_image_size which is perfect for that), so that someone could temporarily unhook that behavior for custom use-cases.

Attachments (1)

44801.diff (5.4 KB) - added by flixos90 6 years ago.

Download all attachments as: .zip

Change History (3)

@flixos90
6 years ago

#1 @flixos90
6 years ago

  • Keywords has-patch added; needs-patch removed

44801.diff addresses the problem as follows:

  • Introduce a filter function wp_constrain_editor_max_image_size_to_content_width() for editor_max_image_size.
  • Outsource all $content_width-related bits from image_constrain_size_for_editor() to that filter function.
  • Hook the filter in with priority 0 in default-filters.php.
  • Clean up the image_constrain_size_for_editor() function's tons of if-clauses a bit to use strict comparisons and less repetitive code for the same thing.

Functionality-wise, this doesn't change anything, I preserved the exact same behavior as is in place currently. However, by having that part being a filter, it can be unhooked to skip it. As a nice side-effect, the very specific $content_width global is no longer used as part of image_constrain_size_for_editor().

#2 @joemcgill
5 years ago

  • Milestone changed from Awaiting Review to Future Release

Hi @flixos90, this seems related to and possibly a duplicate of #35390. Moving this functionality to a filter could help people work around problems with $content_width but I'm unsure if we need to maintain that behavior now that we've moved to the new block editor, which includes the possibility of image widths that extend outside the traditional content area (i.e. align-wide and align-full).

Note: See TracTickets for help on using tickets.