Opened 12 years ago
Closed 12 years ago
#9800 closed Bug (fixed)
Float Panel for Combo Box doesnt listen to resize event
Reported by: | Gary Gilbert | Owned by: | Piotrek Koszuliński |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.0.1 |
Component: | UI : Floating Panel | Version: | 4.0 |
Keywords: | Cc: |
Description
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.
Attachments (3)
Change History (16)
comment:1 Changed 12 years ago by
Milestone: | → CKEditor 4.0.1 |
---|---|
Status: | new → confirmed |
comment:2 Changed 12 years ago by
Owner: | set to Piotrek Koszuliński |
---|---|
Status: | confirmed → assigned |
comment:3 Changed 12 years ago by
Milestone: | CKEditor 4.0.1 → CKEditor 4.0.2 |
---|
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.
- Editor position changed programmatically - impossible to handle in decent way (other than setInterval). Perhaps we should implement editor's method for notifying about that.
- 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).
- When editor is inside scrollable box its position can be changed when scrolling this box - like 1.
- 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.
comment:4 Changed 12 years ago by
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.
comment:5 Changed 12 years ago by
Milestone: | CKEditor 4.0.2 → CKEditor 4.0.1 |
---|
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.
comment:6 Changed 12 years ago by
Status: | assigned → review |
---|
Pushed git:9a47c5d on review.
Continuation on #9803.
comment:7 Changed 12 years ago by
Status: | review → review_failed |
---|
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.
comment:8 Changed 12 years ago by
Status: | review_failed → review |
---|
Pushed code simplification and tests for opening and closing float panel.
Changed 12 years ago by
Attachment: | winFFwtfRandomPanelResisting.ogv added |
---|
comment:9 Changed 12 years ago by
Status: | review → review_failed |
---|
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
comment:10 Changed 12 years ago by
It's not random and not Win only. Second opened float panel isn't properly closed :(.
comment:11 follow-up: 12 Changed 12 years ago by
Status: | review_failed → review |
---|
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.
comment:12 Changed 12 years ago by
Status: | review → review_passed |
---|
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.
comment:13 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Masterised git:979136f.
I couldn't reproduce this on Chrome on FF. Reproducible on IEs.