Opened 7 years ago

Closed 7 years ago

#6387 closed Bug (fixed)

autogrow setTimeout causes problems

Reported by: Dinu Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.5.3
Component: General Version: 3.4.1
Keywords: Cc:

Description

The autogrow timeout to resize the contents causes a couple of problems: 1) Scrollbars appearing (I don't know if this is avoidable?) 2) JS errors when destroying editor (function runs after window is destroyed) (trivial fix attached)

Attachments (4)

autogrow.patch (361 bytes) - added by Dinu 7 years ago.
Patch to test for editor.window before trying to resize
6387_2.patch (1012 bytes) - added by Sa'ar Zac Elias 7 years ago.
6387_3.patch (1.5 KB) - added by Sa'ar Zac Elias 7 years ago.
6387_4.patch (448 bytes) - added by Sa'ar Zac Elias 7 years ago.

Download all attachments as: .zip

Change History (15)

Changed 7 years ago by Dinu

Attachment: autogrow.patch added

Patch to test for editor.window before trying to resize

comment:1 Changed 7 years ago by Dinu

Keywords: HasPatch Review? added; autogrow removed

comment:2 Changed 7 years ago by Garry Yao

Keywords: Review? removed
Status: newconfirmed

comment:3 Changed 7 years ago by Wiktor Walc

Milestone: CKEditor 3.5.2

Ad 1) I assume you're talking about scrollbars that appear for less than a second straight before CKEditor resizes itself?

Ad 2) Confirmed

editor.window is null
ckeditor/_source/plugins/autogrow/plugin.js
Line 13

comment:4 Changed 7 years ago by Dinu

1) Yes, I'm not sure it's avoidable. However, I am currently using the plugin w/o the 100ms timeout; it's faster and I see no glitch, is it an all-browsers must?

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 6387_2.patch added

comment:5 Changed 7 years ago by Sa'ar Zac Elias

Owner: set to Sa'ar Zac Elias
Status: confirmedreview

Considering insertHtml wasn't even listed, and I don't see a glitch with any of the events, we don't need the timeout.

comment:6 Changed 7 years ago by Garry Yao

Status: reviewreview_failed

The timeout must be kept, as we have no guarantee on when actual content will be inserted on those events, the right way to deal with the scrollbar flicking is to persist overflow:hidden to doc.

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 6387_3.patch added

comment:7 Changed 7 years ago by Sa'ar Zac Elias

Status: review_failedreview

comment:8 Changed 7 years ago by Garry Yao

Status: reviewreview_failed

The proposed solution looks quite complicated, I dont think the flicking scrollbar issue desires to fix in this way, so right now let's just have the JS errors addressed.

Changed 7 years ago by Sa'ar Zac Elias

Attachment: 6387_4.patch added

comment:9 Changed 7 years ago by Sa'ar Zac Elias

Status: review_failedreview

comment:10 Changed 7 years ago by Garry Yao

Keywords: HasPatch removed
Status: reviewreview_passed

comment:11 Changed 7 years ago by Sa'ar Zac Elias

Resolution: fixed
Status: review_passedclosed

Fixed with [6428].

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy