search

getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.27k stars 167 forks source link

file::url component doesn't work as expected in 3.6.x on File Panel Pages #4143

Closed thisisfuller closed 2 years ago

thisisfuller commented 2 years ago

Description

The file::url component doesn't return the correct url to the panel in 3.6.x, downgrading to 3.5.x solves the issue.

Expected behavior
The open button and image preview links on the panel should use the returned url set on the plugin / component and not the default. See the URLS below on the screenshots bottom left.

Kirby 3.6.2 Screenshot 3 6 Kirby 3.5.8 Screenshot 3 5

To reproduce

  1. Create a custom file::url component plugin
  2. Navigate to a file page on the panel
  3. Click the open or image preview.
  4. See URL returns as default rather than a modified one via the component
afbora commented 2 years ago

Summary

The new previewUrl() method with 3.6 only uses the file::url component if preview is disabled or page is draft. Otherwise the link is the local file source.

https://github.com/getkirby/kirby/blob/3.6.2/src/Cms/File.php#L755

distantnative commented 2 years ago

I think the main idea here was that the URL in the Panel should point to the actual file, e.g. in content, and not to the media thumb. But maybe we need an additional check whether a custom file::url component was configured. In that case, always use it no matter what.

thisisfuller commented 2 years ago

@distantnative Thanks, Just to add, it's important when you're trying to make a firewall / secure area for files that shouldn't have any public access.

distantnative commented 2 years ago

@thisisfuller I hear you but wondering why this would be the case for something displayed in the Panel, which isn't public access already.

thisisfuller commented 2 years ago

@distantnative Because by default the panel makes media files for the previews / files in the panel, which in turn could potentially be accessed publicly.

afbora commented 2 years ago

But maybe we need an additional check whether a custom file::url component was configured. In that case, always use it no matter what.

@distantnative I like the simple idea but guess we haven't custom/overridden component checker. We can list native components but can't know which ones overridden. Do you have an idea how about checking overrides?

lukasbestle commented 2 years ago

@afbora We could get the actual component and the core component and compare them using ===. They will only be equal if they point to the same closure.

But to be honest I'm not sure if we should really go this route. Wasn't the reason for the current behavior since 3.6 that people were copying the file URLs and pasting them into fields elsewhere? So we wanted the original, non-changing URLs here. For a file::url component as in @thisisfuller's example this will still work, but what about CDN plugins etc.? There we would be back again with the original problem.

bastianallgeier commented 2 years ago

✅