Opened 13 years ago

Closed 13 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 13 years ago.
Patch to test for editor.window before trying to resize
6387_2.patch (1012 bytes) - added by Sa'ar Zac Elias 13 years ago.
6387_3.patch (1.5 KB) - added by Sa'ar Zac Elias 13 years ago.
6387_4.patch (448 bytes) - added by Sa'ar Zac Elias 13 years ago.

Download all attachments as: .zip

Change History (15)

Changed 13 years ago by Dinu

Attachment: autogrow.patch added

Patch to test for editor.window before trying to resize

comment:1 Changed 13 years ago by Dinu

Keywords: HasPatch Review? added; autogrow removed

comment:2 Changed 13 years ago by Garry Yao

Keywords: Review? removed
Status: newconfirmed

comment:3 Changed 13 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 13 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 13 years ago by Sa'ar Zac Elias

Attachment: 6387_2.patch added

comment:5 Changed 13 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 13 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 13 years ago by Sa'ar Zac Elias

Attachment: 6387_3.patch added

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

Status: review_failedreview

comment:8 Changed 13 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 13 years ago by Sa'ar Zac Elias

Attachment: 6387_4.patch added

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

Status: review_failedreview

comment:10 Changed 13 years ago by Garry Yao

Keywords: HasPatch removed
Status: reviewreview_passed

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

Resolution: fixed
Status: review_passedclosed

Fixed with [6428].

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