Issue
The recent bug fix for external files being broken when using the Download link formatter (#2152323: Download formatter does not work with external files like youtube, vimeo and etc.) makes the resulting link use the filename instead of the custom link text set in the administrative UI.
Cause
After examining the code, this is a result of using file_entity_file_is_local() in line 316 of file_entity.field.inc to skip over the external file. This results in not setting $element and using the last resort of theming the link using theme_file_link() - which happens in field_entity.file_api.inc line 266.
Solution
Instead of opting to use the theme_file_link() fallback instead of theme_file_entity_download_link(), I propose that the module move the local file detection into theme_file_entity_download_link() and simply serve up a different $url for the link based on if it's external or internal. The reason for this is because like theme_file_entity_file_link(), theme_file_entity_download_link() is not very different from theme_file_link(). With a few lines of code, we can modify theme_file_entity_download_link() to support external files instead of pre-empting it.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 2276437-file-download-page-support-non-local.patch | 1.44 KB | dave reid |
| #1 | download_formatter-2276437-2.patch | 1.77 KB | pianomansam |
Comments
Comment #1
pianomansam commentedHere is a patch that does what I suggest: It moves file_entity_file_is_local() to theme_file_entity_download_link() and allows the custom download link to still apply.
Comment #2
dave reidComment #3
dave reidHrm, I'd rather we still route remote files through our page callback, so maybe we can fix this in the download page callback itself and redirect to the remote URL instead of doing a file_transfer().
Comment #4
dave reidWrong patch.
Comment #5
pianomansam commentedDave, that's a really nice approach since we can still count downloads and such.
Comment #6
dave reidCommitted #4 to 7.x-2.x.