timber: TimberImage: Incorrect URL returned when image is stored off-server
When calling get_src (either directly or through typecasting a TimberImage instance to a string), the method doesn’t take into account any of the standard WP methods for retrieving the image’s URL (e.g. wp_get_attachment_url or wp_get_attachement_image_src) and instead goes off and does it’s own thing for deriving the URL.
This works fine in what I would expect to be the majority of use-cases, however where other plugins are used to modify the location of files (in my case, moving uploaded files to an S3 bucket and then removing them from the server), this no longer works.
I suppose this is partially an issue with the other plugin, however they correctly apply a filter to the result of the above core method calls that would otherwise allow it to play nicely with other plugins. i.e. Calling wp_get_attachment_url($id) shows the Amazon S3 URL, not the (incorrect) location on the server.
Is there a reason why you went with a custom implementation over using a core method?
As a workaround for my case (and would work for anyone else affected by this issue), I just sub-classed TimberImage and overrode the get_src method:
<?php
namespace MyProject\PostType;
class Image extends \TimberImage {
/**
* @var int
*/
private $iid;
/**
* @param int $iid
*/
public function __construct($iid) {
$this->iid = $iid;
parent::__construct($iid);
}
/**
* @param string|array $size - See http://codex.wordpress.org/Function_Reference/wp_get_attachment_image_src
* @return array|bool|string
*/
public function get_src($size = 'full') {
return wp_get_attachment_image_src($this->iid, $size)[0];
}
}
I’m happy to put a PR up to correct this, but as I don’t fully understand the reasoning behind the current implementation I’m reluctant to do so without hearing from you first.
Thanks (and thanks for the library, it’s awesome!) 😃
About this issue
- Original URL
- State: closed
- Created 10 years ago
- Comments: 35 (10 by maintainers)
Commits related to this issue
- ref #429 -- fixed server location to build-in filter fix — committed to timber/timber by jarednova 7 years ago
- ref #429 -- remove bogus code — committed to timber/timber by jarednova 7 years ago
- ref #429 -- added test for other hooks — committed to timber/timber by jarednova 7 years ago
- Merge pull request #1320 from timber/429/off_server_location ref #429 -- fixed server location to build-in filter fix — committed to timber/timber by jarednova 7 years ago
Note also the width and height fields, which are in the _wp_attachment_metadata. Currently TimberImage.width and .height return nothing for S3 images, even if you override the src, as the dimensions code tries to read the image dimensions from the image file in the filesystem: https://github.com/jarednova/timber/blob/master/lib/timber-image.php#L122
Thus when running WP in a cloud environment it’s impossible to pre-define width= and height= attributes for![]()
in your HTML, which would speed up page rendering.
Probably would be better performance-wise as well to stop reading image files from FS and just read them from the WP attachment object loaded from DB. The attachment metadata is already loaded in get_image_info: https://github.com/jarednova/timber/blob/master/lib/timber-image.php#L156
Hi @connorjburton,
I’m using this plug-in; when a file is uploaded to the media gallery via the wp-admin, it will take the images, move them to an AWS S3 bucket i’ve setup, and delete them from the application server.
I’m not sure about the details of the implementation on that side, but the bucket and URL are saved in a serialized manner, i’m under the impression that their implementation catches the traditional functions for getting attachments in WP and desiralizes to return the correct URL. During this time, the GUID for the image will still point to server itself instead of S3. All the methods used by timber do not take this into account.
I’ve modified, as was suggested,
srcin timber/lib/timber-images to alwaysreturn wp_get_attachment_image_src($this->ID, $size)[0];and it does return the correct URL. Of course, side-effects need to be checked, but it seems to work in all cases from the limited testing i’ve done.new TimberImage(3726);Will return:You can see what i’m talking about the GUID and the S3 URL.
In twigs
{{ Image(ID).src }}returns before the modification:http://localhost:8080/project- name/wp-content/uploads/2016/03/alert-triangle-red-128.pngAfter the modification:http://bucket-name.s3.amazonaws.com/wp-content/uploads/2016/03/alert-triangle-red-128.pngHope this is enough information, if you need anything else let me know.
@tvanantwerp — well that will get our attention! I’m working on #1279 and #1286 right now; so I’d love to see PRs on this (with tests, please)!
@blindstuff That’s pretty brutal whenever I need to hand the site off to the client, “Hey, don’t update your site because the framework I used on it will break the imagery.”
I’m really not sure what to do but the fact that Timber seems incompatible with WP’s official API puts me off of using it again.
This seems like a pretty common issue as well…
I probably should have clarified in my comment that I’m a developer on WP Offload S3 😃 I’m not sure that it’s Timber’s responsibility to make it work with our plugin though, but maybe that’s not what you’re suggesting. I agree that adding a note to the Timber docs may be helpful for some though.
I’m not able to reproduce the same output you’re seeing, nor what I’ve already experienced. Here’s what I’m getting when I test it now:
WP Offload S3 Lite and Timber 1.22 without filter
WP Offload S3 Lite Timber 1.22 with filter from @moreguppy
This is not the same behavior I’ve encountered previously–maybe upgrading to 1.22 made a difference. The result of
post.thumbnail.srcwithout the filter is obviously still wrong. What I don’t understand is why everything else is now correct. Previously, the filter only ever returned the original size from S3 and no filter would never return anything from S3.These are my WP Offload S3 Lite settings in case it makes a difference.
What’s the status of this?
return wp_get_attachment_image_src($this->ID, $size)[0];seems to fix this, but if it were that easy, I’m sure it’d be merged by now. Can a maintainer update us or can we get this merged?It’d be nice if Timber worked with the plethora of WP S3 plugins out of the box.