Opened 10 years ago

Closed 10 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

  1. Open the replace by class example page with Safari;
  2. Click on 'Font Color' button to open panel;
    • Actual Result: The panel size is shrinked.
  3. Click again.
    • Actual Result: Now the panel size is correct.

Attachments (8)

safari4.jpg (111.4 KB) - added by Garry Yao 10 years ago.
Screenshot of Safari4
3879.patch (2.8 KB) - added by Tobiasz Cudnik 10 years ago.
3879_2.patch (3.4 KB) - added by Tobiasz Cudnik 10 years ago.
2009-07-08-145650_1408x1080_scrot.png (107.7 KB) - added by Tobiasz Cudnik 10 years ago.
2009-07-08-145840_1408x1080_scrot.png (195.7 KB) - added by Tobiasz Cudnik 10 years ago.
2009-07-08-145954_1408x1080_scrot.png (188.4 KB) - added by Tobiasz Cudnik 10 years ago.
3879_3.patch (3.3 KB) - added by Tobiasz Cudnik 10 years ago.
3879_4.patch (3.3 KB) - added by Tobiasz Cudnik 10 years ago.

Download all attachments as: .zip

Change History (22)

Changed 10 years ago by Garry Yao

Attachment: safari4.jpg added

Screenshot of Safari4

comment:1 Changed 10 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: newassigned

comment:2 Changed 10 years ago by Tobiasz Cudnik

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 10 years ago by Tobiasz Cudnik

Attachment: 3879.patch added

comment:3 Changed 10 years ago by Tobiasz Cudnik

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 10 years ago by Garry Yao

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 10 years ago by Tobiasz Cudnik

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 10 years ago by Garry Yao

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 10 years ago by Tobiasz Cudnik

Attachment: 3879_2.patch added

Changed 10 years ago by Tobiasz Cudnik

Changed 10 years ago by Tobiasz Cudnik

Changed 10 years ago by Tobiasz Cudnik

comment:7 Changed 10 years ago by Tobiasz Cudnik

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 10 years ago by Garry Yao

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:9 Changed 10 years ago by Tobiasz Cudnik

Resolution: fixed
Status: assignedclosed

Fixed with [3843].

comment:10 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Webkit Review- added; webkit Review+ removed
Resolution: fixed
Status: closedreopened

I've reverted [3843] with [3853] because the second level menu became broken after it (at least with IE7).

Changed 10 years ago by Tobiasz Cudnik

Attachment: 3879_3.patch added

comment:11 Changed 10 years ago by Tobiasz Cudnik

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 10 years ago by Tobiasz Cudnik

Attachment: 3879_4.patch added

comment:12 Changed 10 years ago by Tobiasz Cudnik

Fourth patch fixes IE's onload handler when editor nested in an iframe.

comment:13 Changed 10 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:14 Changed 10 years ago by Tobiasz Cudnik

Resolution: fixed
Status: reopenedclosed

Fixed with [3893].

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