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