helios-ag / FMElfinderBundle

:file_folder: ElFinderBundle provides ElFinder integration with TinyMCE, CKEditor, Summernote editors
MIT License
275 stars 128 forks source link

Fixes #497 mainUrl didn't contain asset path #509

Open JohJohan opened 1 year ago

JohJohan commented 1 year ago

Add missing asset path for main_url to correctly load the assets.

Fixes #497

JohJohan commented 1 year ago

@helios-ag and @UlrichHP when you have time can you check it?

UlrichHP commented 1 year ago

Hi @JohJohan, I tried your solution but it is not working for me.

In my case the assets_path is not defined because the bundles folder is located directly in the public directory.

Your solution seems to make it mandatory to have the bundles folder in another folder.

JohJohan commented 1 year ago

Hi @JohJohan, I tried your solution but it is not working for me.

In my case the assets_path is not defined because the bundles folder is located directly in the public directory.

Your solution seems to make it mandatory to have the bundles folder in another folder.

Okay i can look into that thanks for the feedback. Maybe checking if the 'asset_path' is set might be enough to fix it. I will have a look.

JohJohan commented 1 year ago

I see default value for 'asset_path' is 'assets'. If i understand you correctly you set it to 'null' value?

UlrichHP commented 1 year ago

I see default value for 'asset_path' is 'assets'. If i understand you correctly you set it to 'null' value?

I didn't have to. I left it commented like in the README - Basic configuration

I think the assets_path option never really worked... Even in the controller, it seems it is only used to define a prefix param which is not used afterwards.

JohJohan commented 1 year ago

I see default value for 'asset_path' is 'assets'. If i understand you correctly you set it to 'null' value?

I didn't have to. I left it commented like in the README - Basic configuration

I think the assets_path option never really worked... Even in the controller, it seems it is only used to define a prefix param which is not used afterwards.

@helios-ag can you confirm the I think the assets_path option never really worked? Otherwise we have a option now that we dont support and we should probably remove it.

But i need it because in a project i have my bundle assets in a different folder there not in /assets there in /public/cdn/. So using the asset_path is good to fix that, but actually using the asset_path it will break others that use default like @UlrichHP

What i can do is:

-  $mainUrl = $package->getUrl('/bundles/fmelfinder/js');
+  $path = '/bundles/fmelfinder/js';
+  if ($this->params['assets_path'] !== 'assets') {
+    $path = '/'.$this->params['assets_path'].$path;
+  }
+  $mainUrl = $package->getUrl($path);

So that it wont break it @helios-ag what do you think? Right now like i said it wont work for my project and i pin it to 12.2.1

UlrichHP commented 1 year ago

@JohJohan This solution works for me too :smile: Maybe you can modify the if loop condition to this : $this->params['assets_path'] !== '/' && $this->params['assets_path'] !== 'assets' since in the README, it is the default value ?

JohJohan commented 1 year ago

@JohJohan This solution works for me too smile Maybe you can modify the if loop condition to this : $this->params['assets_path'] !== '/' && $this->params['assets_path'] !== 'assets' since in the README, it is the default value ?

Yeah that is possible but it feels wrong to skip the config parameter, but using it for the default will break it @helios-ag i'm curious what you think about it?

helios-ag commented 1 year ago

@johanadivare seems like prefix not used anymore, in this case prefix can be also eliminated from controller too

JohJohan commented 1 year ago

@johanadivare seems like prefix not used anymore, in this case prefix can be also eliminated from controller too

Okay yes, but how should i fix loading my assets then? Now with the current code it loads https://x.net/bundles/fmelfinder/js/elfinder.min.js (404) while it should be loading https://x.net/cdn/bundles/fmelfinder/js/elfinder.min.js (200) as i have everything in a cdn folder in my public folder

Smanst3r commented 9 months ago

sprintf('/%s/bundles/fmelfinder/js', $this->params['assets_path']) if I set assets_path: '' the result will be //bundles/fmelfinder/js

Is there a reason why it cannot be done without the leading slash? sprintf('%s/bundles/fmelfinder/js', $this->params['assets_path'])

e.g.
assets_path: /path-to-somethere/where-you-want # /path-to-somethere/where-you-want/bundles/fmelfinder/js
assets_path: '' # /fmelfinder/js

And one more: the command elfinder:install option --docroot=public_html should be equal to --docroot= according to this PR?

Thanks for clarifying

JohJohan commented 2 months ago

@Smanst3r First off sorry for late reply!

Is there a reason why it cannot be done without the leading slash?

Your right that isn't necessary and i removed it.

And one more: the command elfinder:install option --docroot=public_html should be equal to --docroot= according to this PR?

In my project i run: elfinder:install --docroot public/cdn

Then i have the following config:

fm_elfinder:
  assets_path: '/cdn'

So no the docroot is different than assets_path right?