file_entity_file_formatter_file_image_view() checks whether an image's stream wrapper is STREAM_WRAPPER_LOCAL before rendering a file with an image style. However, if you have an image stored with a writable but non-local stream wrapper, image_style_create_derivative() is happy to create the derivative--it creates a temporary local version, then moves if to the remote location.

file_entity_file_formatter_file_image_view() should just check whether the image's stream wrapper is readable, not whether it's local. Patch coming in a moment.

CommentFileSizeAuthor
#7 file_entity-1358860-remote_files-7.patch1.04 KBfirebird
#1 1358860-1-file_entity-display_remote_derivatives.patch866 bytesbecw

Comments

becw’s picture

becw commented
Status: Active » Needs review
StatusFileSize
new 1358860-1-file_entity-display_remote_derivatives.patch866 bytes

Here's a patch.

smartinm’s picture

smartinm commented

The patch works on file_entity-7.x-1.x from media module too.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Assigned: Unassigned » dave reid

Assigning for review.

coderintherye’s picture

coderintherye commented
Status: Needs review » Reviewed & tested by the community

This works. I'm sad how long I spent trying to figure this out myself before finally tracking it down to the file_entity module.

I'm going to go ahead and mark as RTBC, though yes there is a huge performance hit. To me the hit is worth it, but perhaps you'll think differently Dave, and if so, I guess we should brainstorm how we can have remote files included without being too much hit on performance.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Issue tags: +sprint, +Media Initiative
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Reviewed & tested by the community » Needs work

This doesn't apply at all with the current code and will need a larger re-roll.

firebird’s picture

firebird commented
Status: Needs work » Needs review
StatusFileSize
new file_entity-1358860-remote_files-7.patch1.04 KB

Here's a similar fix we did to fix the same problem. Applies to the current dev version.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
commented

Looks good. Is this testable?

kimwes’s picture

kimwes commented
Status: Needs review » Reviewed & tested by the community

Had problem with latest dev version of media+file_entity and storage_api. I have set so that storage_api saves images to s3. File entity didn't render images, only showed icons. With patch from #7 the image showed correctly from s3.

ParisLiakos’s picture

ParisLiakos commented
Assigned: dave reid » Unassigned
Status: Reviewed & tested by the community » Fixed

patch was no longer applying, so rerolled and commited
http://drupalcode.org/project/file_entity.git/commit/86580fe

Automatically closed -- issue fixed for 2 weeks with no activity.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Issue summary: View changes
Priority: Normal » Major
Status: Closed (fixed) » Active

Re-opening. Based on the original issue description, I expected this to use SREAM_WRAPPERS_WRITE not STREAM_WRAPPERS_READ. Because if the stream wrapper is not writeable, it cannot have image style derivatives?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

I guess changing to writeable would break using remote_stream_wrapper images. Will need to think about this.

  • ParisLiakos committed 86580fe on 8.x-2.x authored by firebird 
    Issue #1358860 File Entity wont render image derivatives for images...