Opened 10 years ago
Closed 10 years ago
#34532 closed defect (bug) (wontfix)
Responsive images: calculate the image baseurl from the original image `src`.
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 4.4 |
Component: | Media | Keywords: | |
Focuses: | Cc: |
Description
There doesn't seem to be any cases where one image size is in one location, and the rest of the sizes in another. In that case we can use the original image src
to calculate the URLs of the other sizes used in srcset
.
This will keep the same protocol and path as the image that was inserted in the editor (in post_content) or returned from wp_get_attachment_image_src
in wp_get_attachment_image()
(used for galleries, etc.).
Attachments (3)
Change History (11)
#2
@
10 years ago
In 34532.1.patch: also remove the early test for edited images. Not needed if we match the image base name.
This ticket was mentioned in Slack in #feature-respimg by azaozz. View the logs.
10 years ago
#5
@
10 years ago
I have some reservations on this one. It looks to me like there will no longer be a call to wp_upload_dir()
anywhere, which means that we're making the assumption that because one image size is stored in a specific places that all other sizes of that same image are also stored there. That's definitely the case the vast majority of the time, but it's certainly possible to store them separately.
So what happens if we generate a srcset that has some invalid URLs, and add it to an image? At certain sizes does the image disappear? Or does it simply fall back to showing the image that was already there? If the latter, then it's not a huge deal to me. Basically if means that if you store images in a non-standard way, then you need to write some code to make sure of this new functionality but nothing breaks.
If it's the former though, if there's potential of breakage, then I think we need to step back and think about it a bit. Especially since in this case we are modifying existing user data without telling them or requiring any action on their part at all. If existing posts begin to NOT show an image at a particular viewport size, when they worked before, that's a HUGE deal. (I'm not as worried about it, but even without breakage I assume that having any incorrect URLs will cause additional 404s, something that is already not especially light-weight in WordPress.)
#6
@
10 years ago
So what happens if we generate a srcset that has some invalid URLs, and add it to an image?
I think @aaroncampbell brings up a valid concern here. Without a way to respect any filters being placed on wp_upload_dir()
it's possible that we could add invalid urls to the srcset
attribute. The browser has no way of knowing if a source is valid or not during the selection process, which could result in broken images.
#7
@
10 years ago
...it's possible that we could add invalid urls to the srcset attribute. The browser has no way of knowing if a source is valid or not during the selection process, which could result in broken images.
Right. It even looks like the browsers completely ignore the src
attribute and switch to using only srcset
. Images with correct src
but wrong URLs in srcset
show as broken in both Firefox and Chrome, no matter the size/page width.
...we're making the assumption that because one image size is stored in a specific places that all other sizes of that same image are also stored there.
I've been having the same thoughts/concerns. Even when using wp_upload_dir()
we are still making the same assumption. There is no context when this function runs or the filter is applied, so if different image sub-sizes are in different locations, we will still be generating wrong URLs. I'm not even sure if it is possible to have different image sub-sizes in different locations and still be able to insert them in the editor (using the media modal).
On the other hand using wp_upload_dir()
may be "safer" for older posts on sites that have been moved to a new server, or have started using a CDN. It will get all the updated options and can be filtered by plugins and themes.
In that terms thinking it's best to make a patch with the other updates/fixes for #34430, but continue to use wp_upload_dir().
In 34532.patch:
This patch tests if the image file name from the original URL matches the "base" (no extension) file name from image meta. This is a stricter test but wondering if it could fail in some cases (fail would mean no srcset). On the other hand that makes the check for edited file redundant.
We can relax that to only look for the last
/
when calculating the image's URL, but a failure there will be more severe. It might output invalid URLs resulting in missing images.