File browser window doesn't scroll

[Ben]

I am using my own custom file browser with CKEditor 3.2. When I view the file browser in a normal browser window it scrolls fine.

However, when I click the Browse Server button and it opens the file browser in a popup, the scroll bars don't work...

[Jakob]

We're also running into problems with this, so I added a

scrollbars=yes,

to the params in ​/_source/plugins/popup/plugin.js in line 36. I hope this will get fixed with the next release. Of course we could add a scrollable div around our custom filebrowser like ​suggested in the board but since our filebrowser is not only used via CKEditor I actually don't like to put CKEditor-specific changes into our filebrowser code.

[Wiktor Walc]

It looks like we need to add scrollbars=yes or make it configurable.

Another post where this feature was requested: ​http://cksource.com/forums/viewtopic.php?f=11&t=15966

[Charlie]

simply added scrollbars

[Alfonso Martínez de Lizarrondo]

The features for the window.open call should be configurable.

Also, please note that the bug is currently assigned to Tobiasz.

[Frederico Caldeira Knabben]

Not a problem to have others contributing patches, really.

In any case, couldn't we have it in a way that the opened page itself defines whether to have scrollbars or not?

[Charlie]

Sorry, do you mean the caller defines whether or not to have them? Or the page in the opened window?

I believe that as is, adding scrollbars=yes only adds the bars if they are needed (I.E. the page's content is larger than the viewable area). So in a sense, the page which is opened does determine if they are needed using this method.

[Frederico Caldeira Knabben]

Well, the caller is the editor itself, and it could be cumbersome to have a configuration just for that.

Instead, we could leave it flexible, so the opened page (the page in the opened window) defines whether or not scrollbars are necessary (body overflow set to auto/scroll).

[Charlie]

I mean while this is true. I think that borders on unhelpful. For people who don't want to modify their pages (As per original request), it does nothing.

In addition, people (such as myself) migrating from FCKEditor will find this behavior odd, since FCK opened with the scrollbars.

IMHO, it's better to do the opposite, since this will maximise compatibility. Always show the toolbar and if people don't want it, they can make their website smaller (I.E. fit to the window size they open).

[Alfonso Martínez de Lizarrondo]

My proposal would be as follows:

First, remove all the "feature=no" entries. As defined by window.open, if a feature isn't present in the request it defaults to "no".

Second, remove the dependent, modal and alwaysRaised requests because AFAIK, they don't do anything in a normal web page. Some of them used to work in old Firefox versions, but now they don't seem to do anything.

Lastly provide the option to specify the requested features:

options = (editor.config.fileBrowserWindowFeatures || 'resizable=yes') +

[Frederico Caldeira Knabben]

That's a good proposal Alfonso. Let's go ahead with it.

[Charlie]

Updated as per alfonsoml's proposal

[Charlie]

I did it again. I figured while I'm putting this in my code I might as well put the patch here. This is just alfonsoml's proposal. Except I kept scrollbars=yes because I feel very strongly for the reasons I listed above that this is the best default setting.

It should not affect anyone who is not currently complaining about it. In addition, for those who are complaining, it will fix their problem without any setting changes. The extreme minority are those who display websites bigger than the popup size and do NOT want scrollbars. That is the only group who should have to utilize this setting.

Hope that all makes sense!

[Frederico Caldeira Knabben]

Replying to alfonsoml:

First, remove all the "feature=no" entries. As defined by window.open, if a feature isn't present in the request it defaults to "no".

Looking at the [MSDN docs ​http://msdn.microsoft.com/en-us/library/ms536651%28VS.85%29.aspx], that's not true. Some things default to "yes".

For backwards compatibility, I would not touch the current options, eventually removing the old FF stuff, adding the new configuration option.

[Charlie]

Fixed as per fred's description :-P

[Charlie]

Is there any reason a change this small can't make it into the 3.4 release?

[Frederico Caldeira Knabben]

We're working to close the 3.4 right now, so we should not continue adding things to it. Even small tickets take time, because they involve discussion, coding, review, commit, etc.

Also, we still didn't have the final patch up to 40 minutes ago ;) Now things change :)

[Sa'ar Zac Elias]

Just fix the coding style and add documentation for the new configuration before commiting.

[Frederico Caldeira Knabben]

It looks like no one tested that thing... neither comp615, nor me, nor Saare... where is the "editor" variable being coming from? What a shame guys :P

We're really closing the 3.4 this time.

[Sa'ar Zac Elias]

Fixed with [5830].