Setting config.filebrowserImageBrowseUrl results in displaying also Browser Server on links

[Gerd W. Naschenweng]

I am using the latest nightly build and when setting config.filebrowserImageBrowseUrl the "Browse Server" button is displayed on image-info. The "Browse Server" button is also displayed in the Link-tab.

This should not happen. I also specifically set config.filebrowserImageBrowseLinkUrl='' hoping that this will remove the Browse Server on Link but this is not the case.

[Tony]

Ahem; don't you mean :

{
action : 'Browse', 
target: 'Link:txtUrl',
url: ( undefined !== editor.config.filebrowserImageBrowseLinkUrl ) ? editor.config.filebrowserImageBrowseLinkUrl : editor.config.filebrowserBrowseUrl
},
style : 'float:right', 
hidden : true,

Or even maybe :

{
action : 'Browse', 
target: 'Link:txtUrl',
url: ( undefined !== editor.config.filebrowserImageBrowseLinkUrl ) ? editor.config.filebrowserImageBrowseLinkUrl : editor.config.filebrowserBrowseUrl || editor.config.filebrowserImageBrowseUrl
},
style : 'float:right', 
hidden : true,

tobiasz?

Seeing how you'd would generally want to link an image to a file instead of just an image of itself or an entirely different image. _most_ file browsers will default to showing all files using editor.config.filebrowserBrowseUrl and only images when editor.config.filebrowserImageBrowseUrl is accessed by the browser. At least that's the way my implementation works.

Otherwise the patch seems OK without having tested it.

[Tony]

Yeah. Just tested this. I am confirming myself and asking for someone to test and approve my modification to tobiasz's patch because tobiasz's patch removes function original. tobiasz, the first part of your patch against _source/plugins/image/dialogs/image.js should be :

{
action : 'Browse', 
target: 'Link:txtUrl',
url: ( undefined !== editor.config.filebrowserImageBrowseLinkUrl ) ? editor.config.filebrowserImageBrowseLinkUrl : editor.config.filebrowserBrowseUrl || editor.config.filebrowserImageBrowseUrl
},
style : 'float:right', 
hidden : true,

The second part against _source/plugins/filebrowser/plugin.js :

if ( element.filebrowser.action == 'Browse' ) 
{
var url = ( undefined !== element.filebrowser.url ) 
? element.filebrowser.url : editor.config[ 'filebrowser' + ucFirst( dialogName ) + 'BrowseUrl' ] 
|| editor.config.filebrowserBrowseUrl;  
if ( url ) 

Is fine.

[Frederico Caldeira Knabben]

I'm with tony on this. We don't have to change the feature but simply have the undefined check.

[Tobiasz Cudnik]

Second patch fixes first one's property bug and includes Tony's proposition about filebrowser url inheritance (although in reversed priority order).

[Tony]

Having looked a bit deeper, here's what I think _should_ be happening instead :

If the user says that : editor.config.filebrowserImageBrowseLinkUrl === false No link button should appear. Same for if : editor.config.filebrowserSOME_PLUGIN_BrowseLinkUrl === false

If editor.config.filebrowserImageBrowseLinkUrl or editor.config.filebrowserSOME_PLUGIN_BrowseLinkUrl is undefined and not exactly false then the link button should show and open to editor.config.filebrowserBrowseUrl when clicked.

Also, for the second part of tobiasz's patch, the QuickUpload action isn't being patched like this too. Should it be? Thanks, Tony

[Sa'ar Zac Elias]

QuickUpload should also be patched. I also tend to agree with @tony.

[Tobiasz Cudnik]

This is how things work with latest patch, using Image dialog as example:

Image Info tab uses always:

config.filebrowserImageBrowseUrl

Link tab uses following:

config.filebrowserImageBrowseLinkUrl
config.filebrowserImageBrowseUrl

If first one casts to false (we don't need the strict comparison), then Browse Server button is not visible. If it's undefined, used is the second one.

Fix for Quick Upload is now also included.

[Wiktor Walc]

Just to remind:

• filebrowserBrowseUrl enables all "Browse Server" buttons.
• filebrowserUploadUrl enables all "Quick Upload" tabs.

Let's say that both configuration options are used to control the global filebrowser configuration.

What we really need to do here, is to guarantee that when such a global setting is present, user still must have a way to disable/modify each particular place where filebrowser is enabled (wherever possible - at the level of dialogs and/or where filebrowser.url is specified in the element).

The situation at this moment is that it is possible to modify each particular url assigned to the "Browse Server" button:

// make the url in the flash dialog different than filebrowserBrowseUrl
filebrowserFlashBrowseUrl : 'flashbrowse.php'

however, it is impossible to disable this single button with something like:

// hide the "Browse Server" button in the Flash dialog - doesn't work at this moment
filebrowserFlashBrowseUrl : false

Same applies to QuickUpload tabs.

We can change the target url in the Flash dialog with:

// make it different than filebrowserUploadUrl
filebrowserFlashUploadUrl : 'flashupload.php'

but we cannot hide specific upload tab with:

filebrowserFlashUploadUrl : false

I'm setting it to review_failed mainly because the change included in the patch regarding QuickUpload part doesn't make too much sense to me and I believe we could have a more generic solution here, just like I explained above. Apart from adding support to disable "Browse Server" in the "Link" tab of the "Image" dialog with filebrowserImageBrowseLinkUrl, we should be able to disable the "Browse Server" button and/or the Upload tab in selected dialog.

In other words, what we need to add is:

• if editor.config[ 'filebrowser' + ucFirst( dialogName ) + 'BrowseUrl' ] is defined and is set to false, buttons in this dialog should be hidden (...but still config.filebrowserImageBrowseLinkUrl could enable such button back in the Image dialog if config.filebrowserImageBrowseUrl was set to false)
• if editor.config[ 'filebrowser' + ucFirst( dialogName ) + 'UploadUrl' ] is defined and set to false, the tab should be hidden in this dialog.

[Tobiasz Cudnik]

You're right Wiktor, the patch was missing second undefined check, so the inheritance chain is now as follows(for the Image dialog):

config.filebrowserImageBrowseLinkUrl
config.filebrowserImageBrowseUrl
config.filebrowserBrowseUrl

Inheritance is executed only for undefined values.

[Tobiasz Cudnik]

Fixed with [5988].