Opened 9 years ago
Closed 9 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)
Change History (15)
Changed 9 years ago by
Attachment: | autogrow.patch added |
---|
comment:1 Changed 9 years ago by
Keywords: | HasPatch Review? added; autogrow removed |
---|
comment:2 Changed 9 years ago by
Keywords: | Review? removed |
---|---|
Status: | new → confirmed |
comment:3 Changed 9 years ago by
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 9 years ago by
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 9 years ago by
Attachment: | 6387_2.patch added |
---|
comment:5 Changed 9 years ago by
Owner: | set to Sa'ar Zac Elias |
---|---|
Status: | confirmed → 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 9 years ago by
Status: | review → 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 9 years ago by
Attachment: | 6387_3.patch added |
---|
comment:7 Changed 9 years ago by
Status: | review_failed → review |
---|
comment:8 Changed 9 years ago by
Status: | review → 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 9 years ago by
Attachment: | 6387_4.patch added |
---|
comment:9 Changed 9 years ago by
Status: | review_failed → review |
---|
comment:10 Changed 9 years ago by
Keywords: | HasPatch removed |
---|---|
Status: | review → review_passed |
comment:11 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed with [6428].
Patch to test for editor.window before trying to resize