Float Panel for Combo Box doesnt listen to resize event

[Gary Gilbert]

reproduceable in 3.6.2 also

with style or format dropdowns open, resize the window. The position of the dropdown (float panel) is not updated.

This problem is annoying when ckeditor is initialized in a jquery window that enables repositioning, the panel remains floating in the middle of no-where.

suggest closing dropdown on any window/document click event the same as a "normal" combo-box would.

[Piotrek Koszuliński]

I couldn't reproduce this on Chrome on FF. Reproducible on IEs.

[Piotrek Koszuliński]

Initially I thought that this issue is caused by some easily fixable bug in editor blur/resize listener or new listener on window resize.

Unfortunately it's more complex. New listener on window resize is indeed needed, because IEs don't blur editor when resizing window and don't fire any mouse event. But this isn't of course enough - we need to handle cases that could result in editor changing its position according to document.

1. Editor position changed programmatically - impossible to handle in decent way (other than setInterval). Perhaps we should implement editor's method for notifying about that.
2. Editor position changed by resizing by mouse widget like jQuery UI dialog (what should blur editor, but doesn't do that on any browser, perhaps due to prevDef() called on handler).
3. When editor is inside scrollable box its position can be changed when scrolling this box - like 1.
4. Perhaps more cases are possible.

So handling all of that is basically impossible/very hard. Thus, I'm postponing this ticket, because it doesn't fit in 4.0.1 milestone which is already set.

[Gary Gilbert]

Reproducable on Chrome

[Gary Gilbert]

Reproducable on FF 17.0.1

[Gary Gilbert]

As shown in my two attachments, this problem is not restricted to MSIE. The issue is that the float panel is attached to the end of document body, or according to the PanelDefinition the parent, which never seems to be anything but document.body

The position of the element is not changed in relation to the change of editor (which in my case does not change) but in relation to the document body.

The "Simplest" fix, in my opinion, is to close the dropdown when the element loses focus instead of trying to do any fancy repositioning, besides closing on lost focus is a "normal" behavior of a combo box.

[Piotrek Koszuliński]

Hmmm. I was testing this on Linux and it turns out that on Linux, in contrast to Windows and MacOS, blur is fired on float panel when resizing window by dragging border. That's why it was working for me. On other OSes (and on Linux when maximizing window) element doesn't lose focus when window is resized. So listening on blur is not enough (we do this already ​https://github.com/ckeditor/ckeditor-dev/blob/master/plugins/floatpanel/plugin.js#L191).

So back to 4.0.1. We'll add window resize listener to target most common cases. More complex cases, which I mentioned in comment:3 need new ticket.

[Piotrek Koszuliński]

Pushed ​git:9a47c5d on review.

Continuation on #9803.

[Olek Nowodziński]

​1: CKEDITOR.document.getWindow().equals( doc.getWindow() ); // true

​2: bar.on( 'foo', this.hide, this ); is straightforward for other people and your method doesn't save much bytes (if any).

Another thing is that this feature can be tested by opening a panel and observing if it's gone on several events. Regarding editor mode and resize: piece of cake to trigger. However, to create synthetic window resize event, ​document.createEvent can be used (cross-browser code in url). Even if it doesn't work with old IEs (research needed), it's enough to have it tested in modern browsers.

[Piotrek Koszuliński]

Pushed code simplification and tests for opening and closing float panel.

[Olek Nowodziński]

Tests are awesome. However I spotted a random issue that I'm able to reproduce on Win only with IE9, latest FF and IE7. See: attachment:winFFwtfRandomPanelResisting.ogv​​

[Piotrek Koszuliński]

It's not random and not Win only. Second opened float panel isn't properly closed :(.

[Piotrek Koszuliński]

Ha! :) My first version was correct, but I forgot to comment why. We need that floatPanel#hide wrapper due to limitations of events implementation. It's not possible to attach one function twice on the same event to the same object. And floatPanel#hide is in prototype chain so it's shared between all instances.

Pushed revert and test.

[Olek Nowodziński]

Replying to Reinmar:

Ha! :) My first version was correct, but I forgot to comment why. We need that floatPanel#hide wrapper due to limitations of events implementation. It's not possible to attach one function twice on the same event to the same object. And floatPanel#hide is in prototype chain so it's shared between all instances.

Pushed revert and test.

Well done.

[Piotrek Koszuliński]

Masterised ​git:979136f.