When using Media Gallery with the media module and s3fs images are not being displayed. Instead all images were just displayed at file links.
This was due to no image styles being applied when rendering because file_entity_file_is_local() returns FALSE.
The function file_entity_file_is_local() checks the available file systems which advertise as local STREAM_WRAPPERS_LOCAL. I think this is to specific as the functionality does work nicely with the s3fs module and so broadening this criteria to read and write with STREAM_WRAPPERS_WRITE_VISIBLE will be more specific.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | file-entity-file-is-readable-2550231-12.patch | 1.15 KB | JoeMcGuire |
Comments
Comment #2
JoeMcGuire commentedProposed solution attached to detect stream wrappers on the more specific read and write flags instead of local using
STREAM_WRAPPERS_WRITE_VISIBLEComment #3
dave reidThis changes the API, and cannot be changed in this way. We should change file_entity_file_formatter_file_image_view() instead if needed.
Comment #4
JoeMcGuire commentedHi Dave,
Firstly thanks for all your great work on Drupal, I see your name around a lot.
Can you give a bit more insight into which API you are concerned about being broken?
We did review removing the check all together from
file_entity_file_formatter_file_image_view()but concluded that read only file systems would fail ungracefully.The module appears to work very nicely with s3fs once this patch is applied. s3fs describes itself as read and write, but critically NOT being local; which I think is descriptive.
This approach (in theory) just alters the demands on the file system wrapper so that any which can read and write can use the functionality regardless of being local or not.
Many thanks,
Joe
Comment #5
coredumperror commentedThe reason your fix isn't viable is that you're changing what a call to
file_entity_file_is_local()means. It no longer returns what you'd expect based on the name of the function. The right solution would probably be to replace any relevant calls tofile_entity_file_is_local()with calls to a new function that does what your patched version does, e.g.file_entity_file_is_readwrite().Comment #6
JoeMcGuire commentedThanks coredumperror.
That sounds reasonable and I can certainly follow up tomorrow with a more thorough patch as you have described.
Comment #7
dave reidfile_entity_file_is_local() is an API function that determines if the file is local or not. Changing the behavior of that API function would mean we would have to rename it to file_entity_file_is_writeable() or something else, because keeping the name the same would not match what it actually does.
Comment #8
JoeMcGuire commentedThanks Dave for such a prompt response
I'll follow up tomorrow with a more thorough patch inline with you are coredumperrors direction.
Comment #9
dave reidFYI this was fixed in File Entity 7.x-2.x with the following issue: #1358860: File Entity won't render image derivatives for images stored using non-local stream wrappers so I'd probably recommend the same fix.
Comment #10
dave reidOr alternatively taking a look at the current file_entity_file_formatter_file_image_view() function in File Entity 7.x-2.x and copying the changes we need.
Comment #11
JoeMcGuire commentedThanks Dave. I'll aim to mimic to keep consistency.
Comment #12
JoeMcGuire commentedThanks Dave and Coredumperror.
Attached is an updated patch which implements
file_entity_file_is_readable()inline with the 2.x branch fix at #1358860: File Entity won't render image derivatives for images stored using non-local stream wrappersComment #13
chris matthews commentedClosing this issue as outdated. However, if you think this issue is still important, please let us know and we will gladly re-open it for review.
sincerely,
- the Drupal Media Team