Opened 8 years ago

Closed 8 years ago

#3793 closed Bug (fixed)

Combine skin images

Reported by: Frederico Caldeira Knabben Owned by: Tobiasz Cudnik
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: Confirmed Review+ Cc:

Description

Some skin images could be combined inside sprites.png, reducing the number of HTTP calls.

Currently, the following could be considered:

  • arrowdown.gif
  • arrowtop.gif
  • arrowleft.gif
  • arrowright.gif
  • resizer.gif
  • resizer_rtl.gif

Attachments (13)

3793.patch (11.1 KB) - added by Tobiasz Cudnik 8 years ago.
skins.zip (90.9 KB) - added by Tobiasz Cudnik 8 years ago.
Binary files.
3793_2.patch (15.0 KB) - added by Tobiasz Cudnik 8 years ago.
skins_2.zip (85.4 KB) - added by Tobiasz Cudnik 8 years ago.
3793_2 Screenshots.zip (11.3 KB) - added by Frederico Caldeira Knabben 8 years ago.
3793_3.patch (19.4 KB) - added by Tobiasz Cudnik 8 years ago.
skins_3.zip (85.7 KB) - added by Tobiasz Cudnik 8 years ago.
skins_all_4.zip (90.7 KB) - added by Tobiasz Cudnik 8 years ago.
skins_binary_4.zip (43.5 KB) - added by Tobiasz Cudnik 8 years ago.
r3894_IE7.png (146.3 KB) - added by Frederico Caldeira Knabben 8 years ago.
IE7 screenshot of branches/features/3793 at r3894.
2009-07-15-102225_1408x1080_scrot.png (139.5 KB) - added by Tobiasz Cudnik 8 years ago.
IE8
2009-07-15-102228_1408x1080_scrot.png (137.1 KB) - added by Tobiasz Cudnik 8 years ago.
IE7
2009-07-15-102232_1408x1080_scrot.png (122.9 KB) - added by Tobiasz Cudnik 8 years ago.
IE6

Download all attachments as: .zip

Change History (37)

comment:1 Changed 8 years ago by Tobiasz Cudnik

Owner: set to Tobiasz Cudnik
Status: newassigned

comment:2 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added

Changed 8 years ago by Tobiasz Cudnik

Attachment: 3793.patch added

Changed 8 years ago by Tobiasz Cudnik

Attachment: skins.zip added

Binary files.

comment:3 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? removed

Changed 8 years ago by Tobiasz Cudnik

Attachment: 3793_2.patch added

Changed 8 years ago by Tobiasz Cudnik

Attachment: skins_2.zip added

comment:4 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added

comment:5 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

It's almost perfect. Just the following small issues:

  • The arrow for the toolbar collapser is 2px mispositioned in RTL. I've noted that you have used background position 4px for LTR, but 2px for RTL. Maybe using also 4px will fix the problem.
  • IE+RTL: The context menu arrow is at right.
  • IE6: The context menu arrow shows white background when rollover.

I'm adding an attachment with screenshot for all the above.

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: 3793_2 Screenshots.zip added

Changed 8 years ago by Tobiasz Cudnik

Attachment: 3793_3.patch added

Changed 8 years ago by Tobiasz Cudnik

Attachment: skins_3.zip added

comment:6 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

I've managed to fix all 3 mentioned issues plus others below:

  • office2003 dialogs lacked vertical shadow in RTL
  • propagated fix from #3969 in office2003 and v2 skins which turned out to be also affected
  • fixed office2003 and v2 context menu in RTL
  • fixed downarrows in office2003

comment:7 Changed 8 years ago by Tobiasz Cudnik

Instead of #3969 i've meant #3696.

comment:8 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I've followed the following steps:

  1. Applied 3793_3.patch into my updated local trunk copy.
  2. Overwritten my local copy files with all files available at skins_3.zip.

Then, while testing, I've noticed that none of the issues described in my previous comment have been addressed.

Locking closer to the patch, it looks like things have been fixed, but much probably the skins_3.zip file is wrong. so, it was not possible to properly test the fix.

Ideally the skins_3.zip should contain only the binary files that the patch can't have, or even the full directories copy, but including the properly patched files.

comment:9 Changed 8 years ago by Tobiasz Cudnik

skins_3.zip was supposed to provide only binary files, although it contained also normal css files from previous patch, what created the confusion.

I will prepare new zip packages for easier testing.

Changed 8 years ago by Tobiasz Cudnik

Attachment: skins_all_4.zip added

Changed 8 years ago by Tobiasz Cudnik

Attachment: skins_binary_4.zip added

comment:10 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

comment:11 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I've overwritten my local copy with skins_all_4.zip and the layout got totally broken in IE. In Firefox it's also possible to note issues when collapsing the toolbar.

comment:12 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed

It looks like things where broken because skins_all_4.zip is not good for it. The right way is instead having the latest patch plus skins_binary_4.zip.

comment:13 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

Everything looks good, except a rendering issue in the toolbar arrows with IE7+Standards.

comment:14 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

Patch commited with [3890] in features/3793 branch.

IE7 standard incorrectly handles alpha transparency in backgrounds when element itself is filtered with alpha transparency.

comment:15 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The Kama skin looks good. There are issues with the Office2003 skin, with IE+Quirks and IE6.

  • The dialog shadow is broken and not properly rendered (no support for PNG transparency)
  • The dialog close button is mispositioned.

I'll be attaching a screenshot illustrating it in IE7. The same problems can be see in IE6, no matter the mode.

Changed 8 years ago by Frederico Caldeira Knabben

Attachment: r3894_IE7.png added

IE7 screenshot of branches/features/3793 at r3894.

comment:16 Changed 8 years ago by Frederico Caldeira Knabben

Regarding the dialog shadow, the current trunk behavior is the expected one, having no shadow at all in IE+Quirks and IE6.

comment:17 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

Changeset [3896] address issue you've mentioned. Attaching screenshots.

Changed 8 years ago by Tobiasz Cudnik

IE8

Changed 8 years ago by Tobiasz Cudnik

IE7

Changed 8 years ago by Tobiasz Cudnik

IE6

comment:18 Changed 8 years ago by Frederico Caldeira Knabben

I've partially reverted [3896] and properly fixed the close button position with [3905].

comment:19 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I've also fixed the dialog borders in the office2003 skin for IE+Quirks+RTL with [3906].

Now, the V2 skin is broken isntead.

comment:20 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

Review? for [3907].

comment:21 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

Kama: IE+Quirks: The Button Panel items in the toolbar (color selectors and SCAYT) have no arrows anymore.

comment:22 Changed 8 years ago by Tobiasz Cudnik

Keywords: Review? added; Review- removed

Review? for [3921].

comment:23 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

I have made a small fix for v2 with [3923]. Things should be ok now... finally!

comment:24 Changed 8 years ago by Tobiasz Cudnik

Resolution: fixed
Status: assignedclosed

Fixed with [3927] and [3928].

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