Floating toolbar does not reposition vertically

[Jeremie Miserez]

The floating toolbar is not repositioned correctly when the layout of the page changes, e.g. because of:

• browser window resize
• the content above the editable div gets smaller or bigger

Steps to reproduce:

1. Go to ​http://nightly.ckeditor.com/4042/samples/inlineall.html
2. Click on the two middle blocks to show the two toolbars
3. Remove some text from the first block
4. Click on the second block

Expected: The toolbar should have moved a bit upwards Observed: The toolbar is on top of the text and has not moved at all, even though the editable is now at a different position (see screenshot).

Note: Focus/Blur has no effect, the toolbar re-appears at the exact same wrong position.

Reproduced on all browsers tested (Windows: Chrome 23.0.1271.95m, Firefox 10.0.11 ESR, IE8) with CKEditor 4.0 "Full" Release and Nightly v4042.

[Jeremie Miserez]

Screenshot showing the nightly rev 4042 with the bug

[Jeremie Miserez]

Replying to jmiserez:

[Jeremie Miserez]

After looking through the positioning logic in the floatingspace plugin, I think that the mode-based approach might not be ideal. Correct positioning can be achieved with much more consistent results with something like this:

if (editorRect.bottom + dockedOffsetY <= pinnedOffsetY ) {
// outside window
updatePos( 'absolute', 'top', editorPos.y + editorHeight + dockedOffsetY );
} else if (editorRect.bottom - pinnedOffsetY - spaceHeight <= spaceHeight && !(editorRect.bottom <= pinnedOffsetY)) {
// docked bottom
updatePos( 'absolute', 'top', editorPos.y + editorHeight + dockedOffsetY );
} else if (editorRect.top - dockedOffsetY - spaceHeight <= pinnedOffsetY) {
// pinned top
updatePos( 'fixed', 'top', pinnedOffsetY );
} else {
// docked top (default)
updatePos( 'absolute', 'top', editorPos.y - spaceHeight - dockedOffsetY );
}                                                                                                                                     

In addition, the position should be recalculated on both scroll and window resize events, not just scroll events.

[Jan Wloka]

Subscribed.

[Jakub Ś]

Problem can be reproduced from CKEditor 4 beta.

To reproduce:

1. Go to middle column
2. Click in lower div ("Praesent wisi accumsan sit amet nibh")
3. Click in uppder div and remove some text from it
4. Click in lower div again

[Jakub Ś]

Possible duplicate #9899.

There is also h-scroll problem #9903.

[Alexander]

Subscribed

[Jakub Ś]

#9602 was marked as duplicate.

---

Please note that you get the same result when adding dynamic content to page (ckeck out 9602.html)

[Matti Järvinen]

Added myself to cc

[Peter]

cc to myself

[Chris Ingham]

Adding myself to Cc:

[Jakub Ś]

#10059 was marked as duplicate.

[Jakub Ś]

#10075 was marked as duplicate.

[Nick]

cc to myself

[Mike Burgh]

following

[Mindaugas]

still not fixed in 4.1 cc to myself

[Jakub Ś]

Version number is used to indicate when problem stated occurring.

@minds and all other users - stop changing version numbers to show us that issue is still reproducible in latest CKEditor. We are aware of that!!!

[Ben Ng]

Add to cc

[Eduro]

Add to cc

[roelvd]

Add to cc

[roelvd]

Anyone who got a temporary work around / hack?

[Nick]

The workaround I'm using is to reload CKEditor whenever the position needs to change.

[roelvd]

But how do you know that beforehand? Or are you listening to all sorts of events in order to do so?

[Nick]

For me, the position changes when some divs are moved around. When this happens, I reload ALL the editors in there. Depends on your situation. Keep in mind that this reloading tends to take a second or two, depending on the number of editors.

[Eduro]

Any news on this issue? Reloading all boxes is a pretty heavy performance hit and is not an acceptable solution :|

[Claire Martinez]

added to cc

[Olek Nowodziński]

Created branch ​t/9816 with a fix:

• Revisited vertical space positioning.
• Replaced comments with ASCII images of the cases.
• Re-position the space on editor's change event.
• To simplify conditions, re-positioning is performed on each window scroll/resize and editor focus/blur.

[Olek Nowodziński]

NOTE: #9899 is a totally different ticket, touching focus manager. It's because clicking other editor's resizer doesn't trigger blur in the first editor.

[Piotrek Koszuliński]

Repositioning on every #change is IMO too (unnecessarily) heavy. I'd add eventsBuffer (implemented in #9764) to reduce frequency of repositioning.

[Piotrek Koszuliński]

Just to know about what numbers we're talking - I made a test.

IE9 - on master I've got ~12% CPU core usage, on t/9816 it reaches even ~20% for the same typing speed. Chrome. 3% -> 5%. FF 5% -> 7%.

[Olek Nowodziński]

• Ported CKEDITOR.tools.eventsBuffer from #9764.
• Created t/9816 branch in tests: ported tests eventsBuffer tests from #9764.
• Used two separate eventsBuffers: .5s for change event and .1s for other events to avoid delayed re-positioning.
  • Reduced CPU load.
• Re-factorized the entire code of floatingspace:
  • removed awkward order of variable declarations
  • improved variable/function scoping
  • improved performance by moving function definitions out of layout() body
  • fixed code style issues

If the last change seems too drastic, it can be discarded (the last commit in the branch).

[Piotrek Koszuliński]

TODO: Add @since tag to eventsBuffer doc.

[Piotrek Koszuliński]

I pushed two commits:

1. Improvements of eventsBuffer docs.
2. Prevent caching editable. Currently inline editor's editable is not dynamic like framed's one, but since it may change, it's safer take this into account now.

While testing I found that floating space behaves now differently than on master.

Case 1.

On master toolbar would be positioned below editable, on t/9816 toolbar is never positioned below editable.

However, I've never liked the moment when toolbar is repositioned below editable. The condition said - if visible part of editable is lower than toolbar, place toolbar under the editable. It's not optimal in cases like:

There's a lot more available space below editable, than above, so it would make sense to place toolbar below editable.

Case 2.

Toolbar is always visible, even if editable is completely hidden. On master it was pinned to editable.

I think that in this case previous behaviour was correct, because it was less confusing. Toolbar shouldn't be visible, when it cannot be associated with focused editable.

Conclusion

• Case 2. should be fixed.
• We can discuss what to do with case 1., but t/9816's behaviour is not correct.

[Olek Nowodziński]

Updated the branch:

• Case 1: Indeed, this is cumbersome. Now the space is always moved to the bottom of editable when it's possible, e.g. when the space would float over the editable and there's enough space below the editable to have it there.
• Case 2: Reverted master's behavior.

[Piotrek Koszuliński]

I have tested t/9816 and I liked changes very much. Now toolbar is more often positioned below editable (what makes a lot of sense) and thanks to that it is also visible that fix for the toolbar positioning on editor#change works.

I asked also Wiktor to check new behaviour, but for now I'm R+ing.

[Olek Nowodziński]

Merged branch into master, dev (​​git:711cec9), tests (8e4f9dd).