Compared to v.2, there's a change in the way links to file are composed, that radically changes the way LinkIt works.
When I link to a file, now I get always a path like

/file/FID

while in v.2 I used to get

/system/files/FILEPATH

this means that the links lead to a page (not to the file directly) where there's a link to download the file.
Therefore I can't use LinkIt anymore to insert links to files to download.

I tried the 3 different insert methods but no luck, they all insert the path alias "/file/FID"

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

anon’s picture

Status: Active » Closed (won't fix)

Linkit links to enteties, and file_entity is responsible for the /file/FID path.

Nothing I can do about that, sorry.

gmclelland’s picture

Is there any way we can make this an option? Link directly to the file or Link to the file entity?

I was bummed when tried using 3.x on a new site a found out that I can't link to files like I used to in 2.x.

The only other work around I know is to use the https://drupal.org/project/rabbit_hole module's Rabbit Hole's file module to redirect file/%id to the actual file.

gmclelland’s picture

I don't want to hack the module and maybe there is a better way to do this, but It looks like if I add the following code to plugins/linkit_search/file.class.php it goes back to the 2.x way of inserting the path

  /**
   * Create an uri for an entity.
   *
   * @param $entity
   *   The entity to get the path from.
   *
   * @return
   *   A string containing the path of the entity, NULL if the entity has no
   *   uri of its own.
   */
  function createPath($entity) {
    return file_create_url($entity->uri);
  }

I think linkit should provide the same options as https://drupal.org/project/ckeditor_link_file:
The ability to link to files (ex. sites/default/name-of-file.pdf)
File Entity URLs (file/fid) - though I'm not sure why you want this??
or file entity downloads (file/fid/download) will prompt the user

Is there a way to alter/override this class without hacking it?

Please consider adding this functionality back to the main module. The people complaining about relative or absolute links can easily fix those paths with the Pathologic module.

gmclelland’s picture

Here is a patch to restore the 2.x functionality. Just so I remember later.

gmclelland’s picture

Status: Closed (won't fix) » Needs review

Setting to NR for feedback. Please consider still linking directly to files.

Status: Needs review » Needs work

The last submitted patch, 4: restore-linkit-file-paths-2158107.patch, failed testing.

treksler’s picture

IMO, linking directly to files should, at most, be optional.

lias’s picture

I would also request this option. When my users add links to files they are thrown off by the display of the page with the file link. It is more intuitive to link directly to the file itself when used within a document. Otherwise, there's an additional and not necessary page view.

anon’s picture

Version: 7.x-3.1 » 7.x-3.x-dev
Status: Needs work » Needs review
FileSize
1.67 KB

Test this patch.

This patch adds an option to directly link to files.
The option is located in the "Managed files" settings in the search plugin section.

gmclelland’s picture

@anon - Thank you for the patch. It has a quick typo "pluing".

After testing your patch I found that links that are generated will force a download prompt when clicked. Just to clarify, links are generated like this if the media and file entity modules are enabled.
Example: /file/57/download?token=xL_rbLo6kA55I58fiWU4cwY-dox_YeIxnw1VibTwLh4

This is different than linking directly to the file like I did in #4 which would generate links like the previous 2.x branch of linkit did:
Example: sites/default/files/this-is-a.pdf

Maybe there needs to be an additional option if the Media and File Entity modules or Scald are enabled. Maybe something like "Link to file" that generates links like sites/default/files/this-is-a.pdf instead of a link to the file entity or scald atom?

anon’s picture

Status: Needs review » Needs work

The problem with #2 is that file_create_url returns a full uri. We don't want that as it can be an issue when moving from a developer enviroment to production.

If the patch in #9 is not good enough, we will have to find another way to solve this.

gmclelland’s picture

Hmm....#2 works perfectly if using Linkit with the Pathologic module. The insert module had a similar issue when trying to add relative link support. It was solved with the patch at https://drupal.org/comment/6441422#comment-6441422. Maybe we could use a similar approach?

anon’s picture

It might work with Pathologic, but we don't depend on that module anymore, so we have to find another soltuion.

The solution in the patch you linked to should work, Thanks. Let me see what I can do with that.

mallezie’s picture

Category: Support request » Feature request

Noticed some problems / questions while testing this patch.
The token is not added when using the URL raw path method. This is probably another issue. It's because options are not added on this link.
This causes the download link to get to an access denied age.

Another problem is the token needs to be different for authenticated users / logged in users. When you use the download formatter of file entity it generates the link (with token) on the fly. (You can test by compare a download link on an entity page as logged in user vs anonymous user). Linkit generates the token once, and stores it in the body field. (i'm using it with CKeditor in a body field).
This gives a direct download link, which works for the user who created the link, but the link is not working as an anonymous user.

A possible solution is indeed to link to the direct file-url, but this sort of recreates the reason why the tokens where introduced. (As i remember wasn't this beacause of https://drupal.org/SA-CORE-2013-002) and has the move site issue mentioned in #11.

Perhaps a working approach could be not to insert a direct link, but insert the 'download file display' mode from media/file_entity?

mallezie’s picture

using the $conf['file_entity_allow_insecure_download'] = TRUE; in your settings file can be a temporary solution.

sheldonkreger’s picture

If you want a direct download, you need to generate a link with the following scheme:

file/fid/download

file/fid will only point to the File Entity page, which has a direct path to the file as a download. That works but is less convenient.

Then, you need to set file_entity_allow_insecure_download' = TRUE;

Otherwise, your download will be blocked by File Entity module.

Once https://drupal.org/node/2267483 is resolved, you may be able to disable insecure downloads again.

To be clear, the insecure download variable's current implementation causes serious problems (access denied for everybody regardless of perms). This is why you have to set that variable to TRUE for now. You can see my posts here: https://drupal.org/node/1995154#comment-8754905

sheldonkreger’s picture

Just an update:

The bug with File Entity tokens is now resolved in 7.x-2x.-dev. So, you can enable file download tokens if you like. #2267483: Remove user ID in download token generation and allow file download URLs to work across all users

In such a scenario, your download link will look like: file/FID/download?token=xxxxx

This module needs to have a way to embed such links easily. You might explore the methods used by the ckeditor_link_file module.
#2154615: File entity link method Download or URL only links to file/fid

sheldonkreger’s picture

Another thought on this (since I'm pretty familiar with FE and Media) . . .

Using the file/FID/download method is preferable for a few reasons:

1. File Entity can keep track of such instances inside content as long as you hook in and register them in the DB when you save your content (for example, in your input filter module code). Then, when you use the 'replace file' feature, it actually works!

2. File download permissions in File Entity are only triggered when you use links with this structure. Providing direct links to files (sites/default/files/filename.ext) will not trigger FE's permissions implementation. Only Drupal's basic private/public stuff will work in those cases.

3. Unfortunately the problem explained in #11 cannot be avoided with either method, because FID's and URI's are both generated in a way which is not friendly to sync up between production and development databases. (IMO this is one of the fundamental issues in D7 which we just have to live with).

pbuyle’s picture

As a side note, notice that as a temporary solution and/or specific solution you can also implement hook_linkit_search_plugin_entities_alter() to override the LinkIt search plugin for file entity in order to override the createPath method and implement your own custon logic for file link.

gmclelland’s picture

Just posting to let everyone know that I think you can use the linkit module with https://www.drupal.org/project/ckeditor_link and http://drupal.org/project/ckeditor_link_file for the solution to this problem for people who are using Media and File Entity modules.

Here is how it works:
Use the linkit-3.x to insert links to the file entity. ex. file/34

Use the ckeditor_link_file input filter to change those links on the file to either the full file url, file download.

Note: even though ckeditor_link and ckeitor_link_file provides plugins for ckeditor's wysiwyg, you don't have to use them. Just keep using linkit instead.

The ckeditor_link_file input filter will add file usage on all files that are linked. Now, you can track where all your files are used and Drupal won't automatically delete them because they didn't have usage. Previously I was using the http://drupal.org/project/file_lock module to prevent Drupal from deleting files automatically.

Now if I could just find a way to convert all my links like http://www.mydomain.com/sites/default/my.pdf to file/34? Anybody know of module or way to perform that conversion?

Hope that helps everyone.

MiroslavBanov’s picture

Status: Needs work » Needs review

With file_entity @revision >= 6a15501d4215854049bac59ec587ed43bf02133a, and patch #9 applied to linkit, direct links to files work even without setting "file_entity_allow_insecure_download" to TRUE.

The only thing that didn't work is using the LINKIT_URL_METHOD_RAW, as it strips the token from the url.
Here is a patch to make that work as well, but it is a bit of a hack, so I am not sure if it is appropriate.

#10 raised concern that the link should not be for download, but should redirect to the file location. I think a download link is better though.

MiroslavBanov’s picture

Here's the actual patch. See previous comment.

franz.skaaning’s picture

Did this patch work for anyone else? Its not working for us.

dsnopek’s picture

Here is a new patch that combines all of the approaches from above!

It gives you the choice of URL type:

  1. Direct file link
  2. Download file link
  3. Entity view page

Here's a screenshot:

Works in my testing.

Please let me know what you think!

MiroslavBanov’s picture

I like the idea of having these options. But I don't think Direct link should be the default, because it's the most problematic.
In a domain migration, the paths that are stored are:
sites/[original domain]/files/[file name]
Suppose that the files are moved to:
sites/[new domain]/files/[file name]

In a simple situation you will have sites/default/files, or you would solve it with a symlink.
But in a complex multi-site environment like Aegir, it is a problem.

Edit:
Just to note that pathologic doesn't work for link fields, only for textarea fields.

gmclelland’s picture

@MiroslavBanov - if you want truly portable paths then you should follow my suggestion in #20.

In my opinion, when using the Media and File Entity modules your inline links(in text areas) should link to the file entity (file/34) and then use an input filter to change that link to sites/default/nameoffile.pdf. I also talk about this in #2326375: Recommendation to use ckeditor_link_file module.

Portable paths across environments are great, but you also need to be aware of file usage. The way I describe in #20 records file usage when you link to files. This allows you to see what nodes are linking to the file and it also prevents Drupal from automatically deleting files on cron that don't have any file usage.

Because it isn't clear, I want to remind everyone that ckeditor_link and ckeditor_link_file modules can be used with TinyMCE and or other WYSIWYG editors. Your basically just using the input filter from those modules to change the file paths and record file usage. I still use the linkit module 3.x for linking to files because I think it provides a better user experience over the builtin in TinyMCE or CKeditor editor's link dialogs(those provide to many options).

Hope that helps someone.

gmclelland’s picture

I just read your edit as well. I'm not sure if it is the correct way but to handle linking to files using link fields, but I create a "file redirect" content type and use the https://www.drupal.org/project/rabbit_hole module to redirect those "file redirect" nodes to the file that is attached to that node via a file field.

In my "file redirect" content type's rabbit hold settings I add [node:field_redirected_file] and choose a 301 redirect. So if anyone visits a "file redirect" node directly, it will instantly redirect them to the file itself.

So now I use linkit on my link fields to choose a "file redirect" node instead of entering a static path to a file in the link field.

It's not idea, but it works.

Hope that helps

dsnopek’s picture

I like the idea of having these options. But I don't think Direct link should be the default, because it's the most problematic.

@MiroslavBanov: I'd be fine with which ever default the maintainers prefer. I chose direct link simply because it matches LinkIt 2.x - but it's super easy to change! :-)

cboyden’s picture

This patch is working great for us. One question: Would it be advisable to use the built-in schemas public:// and private:// to create the direct file links? Does that give any advantages in access control? Would it be hard to determine which schema to use for any given file?

MiroslavBanov’s picture

@cboyden
I don't think that will work. Link field, and text field don't support file stream wrappers. You would just end up with broken links. Direct links to private files still have access control on them, so I am not sure what you would be trying to achieve.

dsnopek’s picture

One question: Would it be advisable to use the built-in schemas public:// and private:// to create the direct file links?

The public:// and private:// stream wrappers are actually in use: the $entity->uri will likely be either a public:// or private:// URI (or something else if any other stream wrappers are available) which is being transformed into an externally available URL to be inserted by our ::createPath() method. We can't output a URI which uses a stream wrapper, because those are only valid in PHP (unless maybe there is an input filter that transforms them, but I'm not aware of one).

dsnopek’s picture

@gmclelland: I finally got around to trying ckeditor_link and ckeditor_link_file. After some fiddling - it works perfectly! It'll convert /file/FID links into direct links to the actual file, and record the reference on the file_usage table. So, that's totally a usable solution.

However, I'm not crazy about all the additional unused code. ckeditor_link is basically a complete replacement for Linkit, but only for CKEditor. It does all the same stuff: plugin system for linking to different entities, autocomplete, dialog, etc.

What would be ideal is a small module that only provides an input filter to convert /file/FID links into direct links and tracks usage on the file_usage table! Then it could be used with any linking mechanism..

In any case, I think the patch on this issue still has an audience: people who want to make direct links with Linkit and don't care about tracking on the file_usage table.

anon’s picture

I feel like I lost focus on this one, I apologize.

It would be nice if someone that's involved in this issue to make a summary of where we stand, what we have, and if there's still any problems. It would help me much, Thanks.

MiroslavBanov’s picture

@#33 - Here is my (probably flawed) summary.

There are 3 ways to link to file that were mentioned here:

  • Direct link to file URI
  • Link to file/[fid]
  • File download link

Linkit 2.x links to files URIs directly, linkit 3.x changed that to link to file/[fid].

General concerns with linking that were expressed:

  • Linkit 2.x worked with direct links, and some people want exactly the same functionality for 3.x
  • Direct links are problematic if there is domain change to a Drupal site.
  • All types of links could become broken because file_usage is not registered for them.

Summary of patches:

  • Patch #4 restores the old functionality by changing links to direct file URI.
  • Patch #9 changes the links to file download link, which are more portable.
  • Patch #22 improves on #9 by making it work with LINKIT_URL_METHOD_RAW.
  • Patch #24 provides configuration to choose between all of the 3 types of links.

Additionally it is discussed that with other modules it is possible to work around all of the problems and limitations of Linkit 3.x, even without any patching. Some of the modules mentioned: ckeditor_link, ckeditor_link_file, pathologic, rabbit_hole, file_lock.

fabsor’s picture

I had som problems with patch #24, when using sites that exists in a subdir. It seems like this change did the trick to fix that.

dsnopek’s picture

@fabsor: Any chance you could make an interdiff between #24 and #35? That would make it easier to review what your changes were. Here's some documentation on make interdiffs:

https://www.drupal.org/documentation/git/interdiff

Thanks!

stefan.r’s picture

FileSize
621 bytes

@dsnopek here's the interdiff you asked for to #35. This is actually still not the best approach to getting the relative path. I'll post a new patch (which merges the changes from #1845886: Inserting File via WYSIWYG Uses Absolute URL).

stefan.r’s picture

This merges in changes from #1845886: Inserting File via WYSIWYG Uses Absolute URL and makes the patch compatible with file entity 1.x as well.

Patch in #24/#35 looks good otherwise!

dsnopek’s picture

Awesome, thanks! I didn't know that technique for making relative links. I'll try to test the latest version of this patch when I have chance!

stefan.r’s picture

I revised this patch and tested it under the following circumstances:

- Adding a profile at admin/config/content/linkit/add with 'Managed files' enabled.

- Under "Managed files plugin settings" I have tested the following 3 settings, with File Entity 1.x, Media, 2.x and without it File Entity:

Direct file link
Download file link
Entity view page

- Added a link using Linkit in CKEditor and confirmed that all 3 settings worked as expected.

@dsnopek, perhaps the "entity view page" option should be disabled if there is no such thing? Just like we disable the download link? Ie. if $callback == entity_metadata_uri_file when we do:

$info = entity_get_info('file');
$callback = $info['uri callback'];
jgullstr’s picture

I've tested the patch in #40 on a multilingual site. "Entity view page" and "Download file link" both work fine, but "Direct file link" adds the current language code to the path in the format "/$langcode/sites/default/file.pdf". Somewhere along the way, $langcode is prepended to the path. This is happening after createPath() is executed. I will investigate further tomorrow.

stefan.r’s picture

@jgullstr this should fix it :)

anon’s picture

Do we need some kind of update procedure if we commit this (#42)? If not, I feel that is time to commit #42.

jgullstr’s picture

I haven't tried #42 yet, will do tomorrow. Provided it works as expected, conf['url_type'] has to be set to LINKIT_FILE_URL_TYPE_ENTITY on existing profiles to preserve current behavior, as this patch sets the default URL type to LINKIT_FILE_URL_TYPE_DIRECT if i read the patch correctly.

jgullstr’s picture

Patch works great now, thanks stefan.r! Only issue I see with it is that direct downloads are instantly enabled for all linkit profiles when applying patch. Since every new profile should have $this->conf['url_type'] set, changing

$url_type = isset($this->conf['url_type']) ? $this->conf['url_type'] : LINKIT_FILE_URL_TYPE_DIRECT;

to

$url_type = isset($this->conf['url_type']) ? $this->conf['url_type'] : LINKIT_FILE_URL_TYPE_ENTITY;

(row 37 and 92 in patch) should have no other effect than preserving current behavior, right?

stefan.r’s picture

Yes, though it'd be preferable to do this in an update hook to preserve current behaviour for existing installs but use a more sensible default for new installs (ie direct if there is no media/file_entity, and entity if there is).

jgullstr’s picture

Oh, didn't think of that. With that in mind, I agree that an update hook makes sense :)

stefan.r’s picture

Update hook added, along with a more sensible default for new installs.

stefan.r’s picture

Coding standards fix :)

gmclelland’s picture

I'm not sure how to do this, but I wonder if this code should be separated into a file_entity class that extends and overrides LinkitSearchPluginFile? or maybe their needs to be some kind of hook for modules to provide their own custom implementation?

What happens if someone wants to use https://www.drupal.org/project/scald or https://www.drupal.org/project/asset or https://www.drupal.org/project/media_entity(D8) as their media module of choice?

stefan.r’s picture

FileSize
57.07 KB
64.73 KB

@gmclelland I don't think that is necessary, LinkitSearchPluginFile only is relevant to entities of type "file". Asset and Scald entities aren't "files", they are their own entity type:

Link creation of "file" entities can go through either drupal core, File Entity 2.x or Media... which is what this module is concerned with.

As long as we choose the "Entity view page" URL mode, what module to use for file link creation should not be a concern to Linkit, as it is dealt with by the Entity API. Links are created through entity_uri(), which uses the "uri callback" defined in the entity info.

Given this, what kind of possible custom implementations were you thinking of? I am happy to allow for these but I don't really see the need?

gmclelland’s picture

Hmm.. Well this may not be a problem right now, but it does seem like we are adding custom code to support File Entity's custom download method.

I'm thinking that Scald or Asset doesn't yet have a force download function like File Entity does, but they may in the future. If that happens, wouldn't you have to add extra if statements to this module to support those modules's download functions?

stefan.r’s picture

Just to clarify for anyone reading this: Scald/Asset entities are not file entities, they are their own entity types which don't provide URL callbacks for the file entity type. Media and File Entity 2.x however *do* do this, linking to 'media/FILE ID' or 'file/FILE ID'. So the '343' identifier in the 'atom/343' link doesn't refer to the file entity ID but to the atom entity ID.

As such, any custom code for downloads on Scald/Asset entities should probably extend the LinkitSearchPluginEntity class, and not have anything to do with this patch or the LinkitSearchPluginFile class, which ought to deal with file entities only (ie. the "file entities" from Drupal core, which are also used by the Media and File Entity modules when creating their URL callbacks).

Very little of the code in this patch is about downloads and I don't see any clean way of reusing any code from this patch if we were to support Scald/Asset downloads in the future, so I doubt this is a concern.

jgullstr’s picture

Status: Needs review » Reviewed & tested by the community

#49 looks good to me. Thanks!

  • anon committed d25d7c1 on authored by stefan.r
    Issue #2158107 by stefan.r, dsnopek, gmclelland, anon, MiroslavBanov,...
anon’s picture

Status: Reviewed & tested by the community » Fixed

Patch from #49 committed.

Thank you all for the great work.

stefan.r’s picture

Status: Fixed » Needs review
FileSize
655 bytes

Thanks! Sorry to reopen this, but here's a small additional fix.

anon’s picture

Status: Needs review » Fixed

committed the fix. Thanks.

  • anon committed 20b3b9e on 7.x-3.x authored by stefan.r
    Issue #2158107 by stefan.r, dsnopek, gmclelland, anon, MiroslavBanov,...

Status: Fixed » Closed (fixed)

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

The last submitted patch, 9: 2158107-9-linking-to-direct-downloads-of-files.patch, failed testing.

The last submitted patch, 22: 2158107-22-linking-to-direct-downloads-of-files.patch, failed testing.

The last submitted patch, 24: linkit-direct-file-link-2158107-24.patch, failed testing.

The last submitted patch, 35: linkit-direct-file-link-2158107-35.patch, failed testing.

The last submitted patch, 38: linkit-direct-file-link-2158107-38.patch, failed testing.

The last submitted patch, 40: linkit-direct-file-link-2158107-40.patch, failed testing.

The last submitted patch, 42: linkit-direct-file-link-2158107-42.patch, failed testing.

The last submitted patch, 48: linkit-direct-file-link-2158107-46.patch, failed testing.

The last submitted patch, 49: linkit-direct-file-link-2158107-47.patch, failed testing.

Status: Closed (fixed) » Needs work

The last submitted patch, 57: linkit-fix-direct-file-link-2158107-57.patch, failed testing.

dsnopek’s picture

Status: Needs work » Closed (fixed)

Not sure what happened here, but marked back to fixed! :-)

nkraft’s picture

I know this has been closed sometime, but I'm checking to see if a patch like this has been included in the D8 version of the module?

I'm having the same exact problem - using linkit with files points to file/fid instead of to a true link to the filet itself. I find this terribly unusable for clients and what they expect.

thanks