Opened 8 years ago
Closed 8 years ago
#3879 closed Bug (fixed)
[webkit] Color button panel incorrect size on first open
| Reported by: | Garry Yao | Owned by: | Tobiasz Cudnik |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.0 |
| Component: | UI : Toolbar | Version: | |
| Keywords: | Webkit Confirmed Review+ | Cc: |
Description
Reproducing Procedures
- Open the replace by class example page with Safari;
- Click on 'Font Color' button to open panel;
- Actual Result: The panel size is shrinked.
- Click again.
- Actual Result: Now the panel size is correct.
Attachments (8)
Change History (22)
Changed 8 years ago by
| Attachment: | safari4.jpg added |
|---|
comment:1 Changed 8 years ago by
| Owner: | set to Tobiasz Cudnik |
|---|---|
| Status: | new → assigned |
comment:2 Changed 8 years ago by
| Keywords: | webkit Confirmed added; Safari removed |
|---|
Affected are:
- Chrome 2
- Chrome 3
- Safari 3
- Safari 4
It's a bit random to reproduce, but it's easiest in case of Chrome 2.
Changed 8 years ago by
| Attachment: | 3879.patch added |
|---|
comment:3 Changed 8 years ago by
| Keywords: | Review? added |
|---|---|
| Summary: | [Safari] Color button panel incorrect size on first open → [webkit] Color button panel incorrect size on first open |
Patch fixes block.autoSize in floatPanel on webkits and should work always, under one condition - first child have to be at least 1px height.
comment:4 Changed 8 years ago by
| Keywords: | Review- added; Review? removed |
|---|
The patch works, but the polling might not be considered, it seems for me a '100ms' fixed delay when joining the IE fix on L218:
// IE7 needs some time (setting the delay to 0ms won't work) to refresh // the scrollHeight. (#3174) if ( CKEDITOR.env.ie && CKEDITOR.env.version >= 7 || CKEDITOR.env.webkit ) setTimeout( setHeight, 100 );
comment:5 Changed 8 years ago by
Single delay doesn't resolve issue fully, tried that at first. Long delay will work, but in most cases it will be unnecessary long.
Why do you think that "polling might not be considered" ?
comment:6 Changed 8 years ago by
As discussed with Tobiasz, we could try to wait the panel frame fully loaded before the 'showBlock' logic, which will hopefully resolve the issue without a polling or a waiting.
Changed 8 years ago by
| Attachment: | 3879_2.patch added |
|---|
Changed 8 years ago by
| Attachment: | 2009-07-08-145650_1408x1080_scrot.png added |
|---|
Changed 8 years ago by
| Attachment: | 2009-07-08-145840_1408x1080_scrot.png added |
|---|
Changed 8 years ago by
| Attachment: | 2009-07-08-145954_1408x1080_scrot.png added |
|---|
comment:7 Changed 8 years ago by
| Keywords: | Review? added; Review- removed |
|---|
It turns out that panels already provide onLoad function, which is even used for setHeight, but only for Gecko browsers. This is most probably because IE's problem with window.onLoad.
I've fixed IE onLoad (using body) and merged code for all browsers to use setHeight fired by onLoad. This fixes colorbutton issue.
Tests in all browsers available in screenshots.
comment:8 Changed 8 years ago by
| Keywords: | Review+ added; Review? removed |
|---|
Nice patch, at this fix we also avoid the risk of finished the document loading before attach the listener.
comment:10 Changed 8 years ago by
| Keywords: | Webkit Review- added; webkit Review+ removed |
|---|---|
| Resolution: | fixed |
| Status: | closed → reopened |
Changed 8 years ago by
| Attachment: | 3879_3.patch added |
|---|
comment:11 Changed 8 years ago by
| Keywords: | Review? added; Review- removed |
|---|
IE had some issue with CKEDITOR var assigment inside iframe's window when it was moved before document open & write & close. When this assigment was after that, it works.
I've tested on IE 6, 7, 8 using SCAYT suggestions and table context menus (second levels).
Changed 8 years ago by
| Attachment: | 3879_4.patch added |
|---|
comment:12 Changed 8 years ago by
Fourth patch fixes IE's onload handler when editor nested in an iframe.
comment:13 Changed 8 years ago by
| Keywords: | Review+ added; Review? removed |
|---|

Screenshot of Safari4