When implementing simple Image CB I have found a number of bugs (I believe). They are mostly related to non-cropped images (cropped ones are working fine).
When uploaded image pixel size exceeds contentblocks.image.max_upload_height or contentblocks.image.max_upload_width it is rescaled to these max values and saved (as expected), but [[+width]] and [[+height]] placeholders in field template are set to original image size (which is not true anymore)
When I choose from already uploaded images using media browser and image exceeds 800x600px, [[+width]] and [[+height]] placeholders are set to maximum 800x600px (respecting original image aspect ratio). What the heck is 800x600 these days anyway?
When setting Manager thumbnail size in Image field definition (thus trying to use phpthumb for preview), preview in manager is broken, though everything else is working fine, both back-end and front-end. Even temporary preview while uploading is showing fine, but disappears after upload. This is due to media source basePath added to src parameter in phpthumb call. I know I can try changing media source settings to use absolute urls/paths. I am afraid this can break other working components. Using crops for manager thumb works fine as well, but I don’t want force users to use cropper.
Really can’t make this working. Any ideas? I consider all these as bugs. It forces us to use always crops, which is annoying for users (crops work fine, but don’t make default crop without user intervention as I noted in another post).
According third point:
In contrast to image Template Variables, which stores only path relative to Media source basePath (that means without basePath part) and adds that basePath on TV output, CB image type stores entire path, including basePath. This is fine on CB output (you have correct path in [[+url]] placeholder) but breaks manager preview. Call to phpthumb must be without basePath part.
I’ve fixed the width/height being calculated before the resize only for 1.15.0-rc2.
The only 800x600 I can find is in the MODX core, as the default size for the thumb in the media browser. The size of that thumbnail should not have any effect on the actually selected image, that’s just how it gets previewed in the MODX browser. The image size placeholders are set from the actual source file.
Is this on 1.15.0-rc1? That should have resolved issues with the thumb in the manager not showing when it belongs to a different media source than the default.
If it’s still happening for you on 1.15.0-rc1 please provide details about your image field and media source configuration so I can follow along to try and reproduce the problem.
System settings:
default_media_source: 1
setting_contentblocks.image.source: 1
setting_contentblocks.image.max_upload_height: 4096
setting_contentblocks.image.max_upload_width: 4096
Test content block of type “Image with title”:
Template: <img src="[[+url]]" width="[[+width]]" height="[[+height]]" alt="[[+alt]]" title="[[+title]]">
Media source override: None
Directory: images/_contentblocks
Crops: (none)
Manager Thumbnail Size: 150x100
Test image: image.jpg 6000x4000
Results:
image dropped to CB, uploaded correctly to /home/testproject/www/_user/images/_contentblocks/image.jpg, rescaled to 4096x2731
Resulting html: <img src="_user/images/_contentblocks/image.jpg" width="6000" height="4000" alt="" title="">
Incorrect size may change in rc2
image deleted and chosen from already uploaded files. Resulting html: <img src="_user/images/_contentblocks/image.jpg" width="800" height="533" alt="" title="">
As Mark noted, 800x600 is defined in /core/src/Revolution/Sources/modMediaSource.php for filemanager_image_width and filemanager_image_height. If I create system setting filemanager_image_width with value 1000, resulting html changes (after deleting and re-choosing already uploaded image) accordingly to: <img src="_user/images/_contentblocks/image.jpg" width="1000" height="667" alt="" title="">
So you somehow retain image preview size when picking from media browser.
In both previous cases manager thumb is broken, with incorrect call: https://myhost/connectors/system/phpthumb.php?src=/_user/images/_contentblocks/image.jpg&w=150&h=100&zc=1&source=1
Correct call should have src without Media source basePath /_user/: https://myhost/connectors/system/phpthumb.php?src=images/_contentblocks/image.jpg&w=150&h=100&zc=1&source=1
This is default media source and everything else is working fine. Image TVs, images inside TinyMCE, Media Browser and other manager previews, CB cropped images and MoreGallery as well.
Leaving Manager Thumbnail Size empty helps, but it loads entire big image into manager, not a good idea.
Okay it looks like #2 is a 2.x vs 3.x issue. On 2.x, the image_width and image_height would return the original image size. On 3.x, that changed and now returns the filemanager size.
3.x does add a new original_width/height so we need to use that. It looks straightforward enough to use that and fall back to the image_width/height if it’s not set (on 2.x), so consider that fixed in 1.15.0-rc2 as well.
Still need to research why the media source thing doesn’t work (#3)… will review that hopefully tomorrow.
Hello Mark, thanks for your effort.
I can confirm #1 and #2 as resolved.
Unfortunately, #3 is still the same bug. The only difference I can see is missing &source=1 parameter from manager php thumb call. But the media source basePath is still there, resulting in error. Seems that phpthumb is still adding basePath, even if it’s not explicitely defined in call.
Now the manager call is: https://myhost/connectors/system/phpthumb.php?src=/_user/images/_contentblocks/image.jpg&w=150&h=100&zc=1
Should be: https://myhost/connectors/system/phpthumb.php?src=images/_contentblocks/image.jpg&w=150&h=100&zc=1&source=1
Another point: based on changelog, there is a new setting “Open to Directory” that when choosing from existing images should open Media browser in defined Directory. This is not working for me, always opening Media browser in Media source root. Also not remembering last used directory.
The fix was to remove the source parameter, to make phpthumb look from the project root. The problem we had trying to run it with the source, is that we don’t store the source-relative URL - only the full URL, as determined by the media source, which does indeed include your media source base url/path and is what the front-end renders too.
Without the source parameter phpthumb should be looking in MODX_BASE_PATH + provided path for the file. So your MODX base path + /_user/images/_contentblocks/image.jpg which should be correct.
When I saw that you have removed source from phpthumb call, I thought it will be fine, but it’s not. Seems that default media source base path is added anyway.
I even tried to clear default_media_source system setting, didn’t help. Seems that media source 1 is always default one. Can see this in modMediaSource: $defaultSourceId = $xpdo->getOption('default_media_source', null, 1);
I tried calling phpthumb on image from another media source and to make it work I had to add source=3 to make it work. But for images from media source 1 (which is also default one) it doesn’t matter if you add source param or not.
Image template variables store path relative to media source. This is why you see correct preview in manager, it doesn’t contain media source base.
But when TV is used (processed), it returns complete path including base, so front end is fine.
Yes, the point is that we’re using the full URL just like an image TV would use. So that phpthumb can get to it regardless of what source it was selected from, as we don’t always know reliably what source an image came from. We just know the full URL.
So the question I don’t have an answer to yet is why passing the full URL does not let phpthumb create the thumbnail for you. That works here, regardless of which source I select an image from.
Is the full URL it generates when selecting an image not correct? If so, that could point to a misconfigured media source. Or maybe there is another variable we’re overlooking here.
images/image.jpg is also what is stored in modx_site_tmplvar_contentvalues db table.
When MODX renders [[*myTV]] on front-end, it adds media source base path, from media source defined in TV Media Sources tab for each context. So one TV can behave differently in different contexts.
One must be carefull when using raw db values of image TVs and not processed values from parser.
Full url in our setup described above is /_user/images/image.jpg
Manager call for thumbnail doesn’t contain media source base /_user/: /connectors/system/phpthumb.php?w=400&h=400&aoe=0&far=0&f=png&src=images%2Fimage.jpg&wctx=web&source=1&version=28d95d9b
Is the full URL it generates when selecting an image not correct?
Full URL is correct, but it must not be full url to work properly.
When playing with direct calls to phpthumb, ti works like I have written before: src must not contain media source base and source if not specified is defaulted to 1.
So the question is why it works differently in your setup?
The funny thing here is that we use CB for years with pretty large and complex sites and we found this bug only recently. It’s because design of those sites required fixed aspect ratios for images, so we made users use crops. Crops works fine for manager previews. Now client requires mostly uncropped images so we are in trouble.
on line 64 sets Media source from request source param: $this->getSource($this->getProperty('source'));
getSource() sets requested or default Media source: $this->source = modMediaSource::getDefaultSource($this->modx, $sourceId, false);
on line 70 calls Media source to process $src param: $src = $this->source->prepareSrcForThumb($src);
If this returns empty, php thumb quetly exits. This is what is happening.
Class modMediaSource.php
function prepareSrcForThumb($src) uses FileSystem class to check if file exist if (!$this->filesystem->fileExists($src)) { return ''; }
It does not, because of how filesystem works and filesystem adapter is initialized
Class modFileMediaSource.php
in initialize() function creates LocalFileSystemAdapter($this->getBasePath(), ... )
getBasePath() returns absolute path for this source
so adapter is doing all operations relative to this base path
Class LocalFileSystemAdapter.php
in constructor creates PathPrefixer using base path that is used later in all operations of this FileSystem instance
All this is consistent with my observations of how it works. So question really is how can be this working differently for Mark?
I understand, but we can’t do that. We store the full url. We haven’t reliably stored the right media source ID when people have selected images to use in the past, plus users can also add images just by entering a URL, or uploading directly which then gets saved somewhere, so moving to the model that uses the media sources API would be tricky for the thousands of existing sites and that existing content.
Did you perhaps change the source with ID 1 to a different base path? On my site(s), that’s always the root one (typically for admin one) which would be consistent with what I’m seeing and what’s different for you.
Development is mainly done on MODX 2 here as well so that could be something that explains the difference, worth reviewing.
Well, that explains everything. If you always have Media source with ID=1 at site root, this works fine. But you need to be carefull and restrict normal users from that source.
We are always creating only media sources needed for users, so they are not from root. As admins, we have other means to access all files on server.
In theory I can try to reconfigure server as you have, it’s probably not too difficult. But this should be mentioned somewhere in docs to spare us from such headache. I don’t think this is specific to MODX2 in general, but to introduction of Media sources. Due to your default setup you didn’t found this glitch until now. Neither we did because of using crops.
Thinking about it further: inspiration might be popular phpthumbof: it’s doesn’t care about Media source, because it’s mostly used as output filter to processed TV (that already include base path). Looking into code, (when useResizer is off) it uses modPhpThumb class directly to generate thumbnail, which also manages caching. It doesn’t call PhpThumb processor that makes things complicated due to Media sources. This would only affect manager thumbnails. It has nothing to do with CB template parsing or front end. So it’s won’t break any site.
@mhamstra I can confirm reconfiguring Media Source ID 1 to website root resolves issue. Ordinary users must not have access to this Source, it might be dangerous.
This also means creating another Source replacing original one, and use that Source everywhere: image TVs, default media source in core system setting, CB system settings, Formit system settings etc.
I consider this as emergency workaround, while it requires Media Source ID 1 to be configured for website root. That Media Source is not needed in any other way and can be even security risk.
Please think about it again: you said we store full url without Media Source ID. This is not exactly true: Media Source ID is stored in contentblocks.image.source and can be overwritten per Image CB Field. You use that Media source each time uploading or choosing existing one. So why not using the same info to construct Media Source relative url in manager thumb?
Correct - that’s basically the first thing I do in a typical site, configuring specific sources for specific uses, but I suppose not everyone does that with a new source.
That’s basically how it was before - we were passing the url and either the fields’ source or the contentblocks.image.source system setting value. But then apparently that stopped working for whatever reason…
I guess we’re going to have to build our own phpthumb endpoint that tries to parse out the base url of whatever media source and hope for the best. Don’t think we can do that client-side which is where most of this logic is at.
This is very broken, conceptually that seems like it’s supposed to be checking for absolute files and let those through, but it only let’s an empty string through - meaning it just checks /.
