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