Opened 5 years ago

Closed 5 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 5 years ago.
11097.html (32.4 KB) - added by Piotrek Koszuliński 5 years ago.
11097-maxHeight.html (32.5 KB) - added by Piotrek Koszuliński 5 years ago.

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by Teresa Monahan

Attachment: largeTable.txt added

comment:1 Changed 5 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 5 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 5 years ago by Piotrek Koszuliński

Attachment: 11097.html added

Changed 5 years ago by Piotrek Koszuliński

Attachment: 11097-maxHeight.html added

comment:3 Changed 5 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 5 years ago by Piotrek Koszuliński (previous) (diff)

comment:4 Changed 5 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 5 years ago by Olek Nowodziński

cc

comment:6 Changed 5 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.3.2
Status: reviewreview_passed

comment:7 Changed 5 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 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy