Opened 10 years ago

Closed 10 years ago

#11097 closed Bug (fixed)

IE: Delay typing text when editor contains large tables and autogrow is enabled

Reported by: Teresa Monahan Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.3.2
Component: General Version: 3.6.2
Keywords: IBM IE Cc: Satya Minnekanti, Irina, peter

Description

To reproduce:

  • Remove margin: 20px; from the body styles in contents.css
  • Open the autogrow sample in IE.
  • Paste the table in the attached file into the source view of the editor.
  • Switch back to wysiwyg mode and type content into one of the table cells.

Problem: There is a delay entering the typed characters into the editor.

This delay does not happen if you resize the table to be wider than the editor (i.e. introduce a horizontal scroll bar) or if you remove the empty paragraph from the end of the editor contents.

This is reproducible in IE8, IE9 and IE10

Attachments (3)

largeTable.txt (127.1 KB) - added by Teresa Monahan 10 years ago.
11097.html (32.4 KB) - added by Piotrek Koszuliński 10 years ago.
11097-maxHeight.html (32.5 KB) - added by Piotrek Koszuliński 10 years ago.

Download all attachments as: .zip

Change History (10)

Changed 10 years ago by Teresa Monahan

Attachment: largeTable.txt added

comment:1 Changed 10 years ago by Jakub Ś

Keywords: IE added
Status: newconfirmed
Version: 4.2.3 (GitHub - master)3.6.2

I was able to reproduce this problem from CKEditor 3.6.2 rev. [7244].

NOTE: I had only problems in IE8 and IE9 (it has worked fine for me in IE10)

comment:2 Changed 10 years ago by Teresa Monahan

Can you see anything in [7244] that may have caused this issue? Can this be targeted for an upcoming milestone? This was reported by a customer so it is a high priority for us.

Changed 10 years ago by Piotrek Koszuliński

Attachment: 11097.html added

Changed 10 years ago by Piotrek Koszuliński

Attachment: 11097-maxHeight.html added

comment:3 Changed 10 years ago by Piotrek Koszuliński

Owner: set to Piotrek Koszuliński
Status: confirmedreview

I pushed t/11097 with dozens of improvements.

First I focused on overall code improvements. The previous implementation was very wasteful - there was no caching at all so e.g. twice on every key press there was a marker element created, many native properties were accessed (including creating wrappers for them) and some of them weren't then used even once. So, the implementation was bad for GC and unnecessary heavy. I refactored those things, but as I predicted, that hasn't fixed the issue, because it was in the root of the algorithm.

The algorithm consists of two resizeEditor() pass executed on every keypress. The second one is needed to reduce height, because after first resize scrollbar may be removed, making editable wider, so e.g. text layout may be changed.

The problem with the table is that when scrollbar appears (after character was typed), editable's width is reduced and table layout has to be recalculated what's heavy even for native implementation (it's much heavier than normal text recalculation). Then, editor is resized first time, so scrollbar disappears and editable becomes wider and table layout has to be recalculated again.

When autogrow_maxHeight is not set, we know that we don't want to display scrollbar at all. So if we set overflow-y to hidden at the beginning, scrollbar won't appear and this way we saved two table recalculations on every key press.

Additionally, since there's no scrollbar we don't need the second resizeEditor() pass at all. And finally, there's no blinking scrollbar when typing.

Summary

The t/11097 branch fixes few issues:

  • performance issue mentioned in this ticket,
  • blinking scrollbar (#7852, fixed only when maxHeight is not set),
  • [IE8] no scrollbar when maximizing editor (there was removeStyle( 'overflow' ), but should be more precise removeStyle( 'overflow-y' )).

Many of the code improvements included in t/11097 are not required for the above fixes, but it was a time to do something with that wasteful code.

Last edited 10 years ago by Piotrek Koszuliński (previous) (diff)

comment:4 Changed 10 years ago by Piotrek Koszuliński

PS. When autogrow_maxHeight is set, the scrollbar does not appear and disappear when typing and resizing in very big table (which means that max height was exceeded), so there's no heavy table layout recalculation. However, when the content does not yet exceed maxHeight the scrollbar blinks. In my opinion it can be fixed by setting overflow-y to none if content does not yet exceed maxHeight and resetting the style only if it does. But this means some deeper changes in the algorithm, which I didn't want to include.

PPS. You may experience odd scroll jump issues in my attachments. They happen because if you start typing in one column, the other get narrower what may cause text wrapping and very big table height change. It is not a bug.

comment:5 Changed 10 years ago by Olek Nowodziński

cc

comment:6 Changed 10 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.3.2
Status: reviewreview_passed

comment:7 Changed 10 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Fixed on master with git:70d9474.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy