Opened 6 years ago

Closed 5 years ago

#6387 closed Bug (fixed)

autogrow setTimeout causes problems

Reported by: dinu Owned by: Saare
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 6 years ago.
Patch to test for editor.window before trying to resize
6387_2.patch (1012 bytes) - added by Saare 5 years ago.
6387_3.patch (1.5 KB) - added by Saare 5 years ago.
6387_4.patch (448 bytes) - added by Saare 5 years ago.

Download all attachments as: .zip

Change History (15)

Changed 6 years ago by dinu

Patch to test for editor.window before trying to resize

comment:1 Changed 6 years ago by dinu

  • Keywords HasPatch Review? added; autogrow removed

comment:2 Changed 6 years ago by garry.yao

  • Keywords Review? removed
  • Status changed from new to confirmed

comment:3 Changed 6 years ago by wwalc

  • Milestone set to 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 6 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 5 years ago by Saare

comment:5 Changed 5 years ago by Saare

  • Owner set to Saare
  • Status changed from confirmed to review

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 5 years ago by garry.yao

  • Status changed from review to review_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 5 years ago by Saare

comment:7 Changed 5 years ago by Saare

  • Status changed from review_failed to review

comment:8 Changed 5 years ago by garry.yao

  • Status changed from review to review_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 5 years ago by Saare

comment:9 Changed 5 years ago by Saare

  • Status changed from review_failed to review

comment:10 Changed 5 years ago by garry.yao

  • Keywords HasPatch removed
  • Status changed from review to review_passed

comment:11 Changed 5 years ago by Saare

  • Resolution set to fixed
  • Status changed from review_passed to closed

Fixed with [6428].

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