WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 7 weeks ago

#21819 assigned enhancement

Use an image size for custom headers instead of duplicating an attachment

Reported by: koopersmith Owned by:
Milestone: 4.9 Priority: normal
Severity: normal Version: 3.5
Component: Customize Keywords: needs-patch
Focuses: Cc:

Description

The improvements in #21810 will allow us to use an image size for custom headers, and we definitely should — it results in fewer attachments, allows the user to recrop the header, and allows us to make intelligent decisions regarding how to treat custom headers when the theme is changed.

Change History (24)

#1 @nacin
5 years ago

  • Owner set to nacin
  • Status changed from new to assigned

#2 @nacin
5 years ago

  • Milestone changed from 3.5 to Future Release
  • Owner nacin deleted
  • Type changed from task (blessed) to enhancement

#4 @SergeyBiryukov
3 years ago

  • Component changed from Media to Appearance

This ticket was mentioned in Slack in #core-customize by valendesigns. View the logs.


2 years ago

#6 @celloexpressions
2 years ago

  • Keywords needs-patch added

The current way that attachments are duplicated can result in some strange experiences for users, especially if the cropped images are significantly cropped, but then appear to be the originls in the media library and accidentally get inserted into posts or elsewhere, or reused with other themes (especially an issue with Twenty Fifteen's portrait featured images).

However, we still need a patch here, and at least some movement on dependency tickets, to work out a solution.

#7 @celloexpressions
16 months ago

  • Keywords close added
  • Milestone changed from Future Release to Awaiting Review

We now use the approach of creating duplicated attachments for all cropped image controls in the customizer, including site icon and custom logo in core. It seems to be working okay, and there are some ways to improve the experience, such as displaying the context of the cropped images in the media library. We should probably maybelater this, and revisit if we're able to actually tackle this from a technical perspective in the future.

This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.


6 months ago

#9 follow-up: @westonruter
6 months ago

  • Keywords close removed

Technically it seems like we could solve it by adding the new crop among the stored intermediate image sizes associated to the original image, as opposed to creating a new attachment. And then custom headers, custom logos, and site icons can all pick out the size that they need. The challenge there could be then picking from between two identical sizes. Perhaps there could be a flag among the sizes to indicate if the crop was made for custom logo, custom header, or site icon to eliminate the guess work.

Last edited 6 months ago by westonruter (previous) (diff)

#10 @westonruter
6 months ago

  • Milestone changed from Awaiting Review to 4.7.3

#11 in reply to: ↑ 9 @joemcgill
6 months ago

Replying to westonruter:

Technically it seems like we could solve it by adding the new crop among the stored intermediate image sizes associated to the original image, as opposed to creating a new attachment.

There are other use cases for WordPress to define a data structure for handling variations on a single attachment, which should be kept in mind here. We would want to consider create multiple image sizes associated with a crop so we could do responsive images and not cross-pollute between different crop types.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


5 months ago

#13 @westonruter
5 months ago

  • Milestone changed from 4.7.3 to 4.7.4

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.


5 months ago

#15 @westonruter
4 months ago

@joemcgill could you elaborate?

Let's talk about the use case for a custom logo. Let's say the logo requires a 400x400 image, but we upload a 640x480 image. The user performs the crop and then… you're saying that there could very well be multiple 400x400 crops of an image that capture different parts of the image (e.g. center,center vs top,left) and so each should be able to co-exist?

What if the cropped image created a new ad hoc image size on the fly and gave it a name like, custom_logo-400x400 and then stored it among the _wp_attachment_metadata for the main attachment. The then problem is that the different responsive image sizes for this 400x400 image would not be created, unless we were to loop through all of the theme's image sizes that are smaller than 400x400 and create a similar ad hoc image size for each, e.g. custom_logo-{$image_size}.

This also then crosses into similar territory with #36441, and what happens if the user uploads a square 512x512 image when a 400x400 image is required. They shouldn't have to crop it since it's already square. But then could we create an ad hoc custom_logo-400x400 image size?

All of this is closely related to responsive image support for images in the customizer media control (#36191).

#16 follow-up: @joemcgill
4 months ago

@westonruter happy to elaborate, apologies for the quick reply and run earlier.

There are two closely related issues related to responsive image support that concern me about how this is implemented.

First, when someone displays an image, we add responsive image support in wp_calculate_image_srcset() by looping through all intermediate sizes in the attachment metadata array and including any image that matches the aspect ratio of our src image, as long as it was created during the same edit as the src file (by searching for the existence of an edit hash). So imagine you upload a file to a blog post and WordPress creates a 150x150 thumbnail cropped from the middle. Later, you decide you want to make this image your custom logo, but want to crop it from top at 400x400. Unless WP knows that these are from different edits when calculating srcset attributes, both could end up being selected, which could mean different crops for the same image depending on your screen size and/or display density. Gross. In short, we need a way to make sure custom crops are treated as a separate edit from preexisting intermediate sizes.

Second, to make sure we include responsive image support for custom logos, it would be nice to generate additional intermediate sizes that relate to the crop being made for the logo. So for instance, if the original image is 2000x1200 and the custom crop is square, it would be nice to generate a series of intermediate sizes that were 1200x1200, 900x900, 450x450, and 200x200 (for example). I'm not sure how we decide what these sizes should be at this point, or if we should just use the registered intermediate sizes.

The couple ideas I have for this off the top of my head would be to extend the sizes property in attachment meta so that each named size could include a sub-array of sizes. Like:

'sizes' => [
    'thumbnail' => [
        'file' => 'path-to-thumb.jpg',
        'width' => 150,
        'height' => 150,
    ],
    'medium' => [
        'file' => 'path-to-medium.jpg',
        'width' => 300,
        'height' => 250,
    ],
    'custom_logo' => [
        'small' => [
            'file' => 'path-to-small-logo.jpg',
            'width' => 200,
            'height' => 200,
        ],
        'medium' => [
            'file' => 'path-to-medium-logo.jpg',
            'width' => 400,
            'height' => 400,
        ],
    ],
]

Alternately, we could add a new property like alternate_sizes which contain sets of edited versions of an attachment.

'sizes' => [
    'thumbnail' => [
        'file' => 'path-to-thumb.jpg',
        'width' => 150,
        'height' => 150,
    ],
    'medium' => [
        'file' => 'path-to-medium.jpg',
        'width' => 300,
        'height' => 250,
    ],
],
'alternate_sizes' => [
    'custom_logo' => [
        'small' => [
            'file' => 'path-to-small-logo.jpg',
            'width' => 200,
            'height' => 200,
        ],
        'medium' => [
            'file' => 'path-to-medium-crop.jpg',
            'width' => 400,
            'height' => 400,
        ],
    ],
    'custom_header' => [
        'small' => [
            'file' => 'path-to-small-header.jpg',
            'width' => 600,
            'height' => 200,
        ],
        'medium' => [
            'file' => 'path-to-medium-header.jpg',
            'width' => 1200,
            'height' => 400,
        ],
    ],
]

Last idea (and probably the most straightforward) is to create a new attachment for custom logo/header crops, like we're currently doing, and set the post parent to the original attachment to keep the association. To keep from having duplicate images in the media library, we might be able to set some metadata on the parent attachment, which indicates that the file should be hidden from the media library if the file was uploaded from one of these upload and crop controls.

Hope this is helpful.

#17 in reply to: ↑ 16 ; follow-up: @westonruter
4 months ago

Replying to joemcgill:

Alternately, we could add a new property like alternate_sizes which contain sets of edited versions of an attachment.

This makes a lot of sense to me. It would eliminate having to keep track of two different attachment posts. Maybe instead of alternate_sizes it could be variations and then this could include a name (e.g. custom_logo) and then the value could contain the width, height, whether cropping was performed done, and so on. Essentially these would be ad hoc image sizes that would be specific to this one attachment.

When opening the media manager, we'd need to pass in a specific variation name as being desired so that when an image is cropped it gets routed to that variation instead of creating a new attachment. It would be nice if, when re-selecting a previous image that was cropped for a given variation, if the cropper tool opened with the original full size image shown but with the cropper configured to have the same bounding box as was previously selected. If they Skip Crop, then the variation would be unchanged. If they Crop, then it would override the previous variation stored in meta and create a new file on disk.

Last idea (and probably the most straightforward) is to create a new attachment for custom logo/header crops, like we're currently doing, and set the post parent to the original attachment to keep the association. To keep from having duplicate images in the media library, we might be able to set some metadata on the parent attachment, which indicates that the file should be hidden from the media library if the file was uploaded from one of these upload and crop controls.

So essentially this would be introducing the concept of derived images. It does seem a bit tricky in the media select frame what to do with the original vs the derived. Also, we'd need to figure out what to do when the original image is deleted. I was thinking this would be more feasible at first, but I like the idea of alternate_sizes/variations more.

#18 in reply to: ↑ 17 @joemcgill
4 months ago

Replying to westonruter:

So essentially this would be introducing the concept of derived images.

In short, adding support for derived images is exactly how I would think about this feature. The main decision I see is determining how we would store/retrieve data about a derived image and how those derived images relate to the original attachment they were derived from.

Something like alternate_sizes or variations on the attachment metadata could be simpler, but I do think there are times where we would want a variation of an image to be treated like a separate image entirely. For example, if we used this same structure to store a variation on an image that was treated by a filter, we might want both the filtered and unfiltered variation visible in the media library.

Also, regardless of the storage mechanism for each named variation, we should create sets of sizes like custom_logo containing its own small, medium, large, etc. sizes. That way we can add responsive image support. For that, I'm not sure if we should just use the default registered intermediate sizes for the site, or if alternate sizes for a custom logo variation should be registered when someone adds support for custom logos—as an additional setting.

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


4 months ago

#20 @swissspidy
4 months ago

  • Milestone changed from 4.7.4 to 4.8

Moving this to the more realistic 4.8 milestone as mentioned during today's bug scrub.

This ticket was mentioned in Slack in #core-media by melchoyce. View the logs.


3 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


3 months ago

#23 @jbpaul17
3 months ago

  • Milestone changed from 4.8 to 4.8.1

Punting to 4.8.1 per bug scrub earlier this week.

#24 @westonruter
7 weeks ago

  • Milestone changed from 4.8.1 to 4.9
Note: See TracTickets for help on using tickets.