Last Comment Bug 517469 - Port Bug 363130: make menuitem icons 16x16px consistently on Windows/Linux.
: Port Bug 363130: make menuitem icons 16x16px consistently on Windows/Linux.
Status: RESOLVED FIXED
: modern
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a3
Assigned To: Philip Chee
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-18 07:05 PDT by Philip Chee
Modified: 2010-07-25 19:24 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v1.0 Use width/height instead of min-width/min-height. (894 bytes, patch)
2009-09-18 07:21 PDT, Philip Chee
neil: superreview-
Details | Diff | Splinter Review
Patch v1.1 Additional fix for editorDialog.css (4.68 KB, patch)
2009-09-20 10:55 PDT, Philip Chee
no flags Details | Diff | Splinter Review
Patch v1.2 Remove redundant styles. (6.12 KB, patch)
2010-07-20 01:21 PDT, Philip Chee
neil: review+
neil: superreview+
Details | Diff | Splinter Review

Description Philip Chee 2009-09-18 07:05:30 PDT
While working on the Lightning integration I noticed that one of the menu items was significantly taller than the others in Modern but not in the Default theme. This was due to lightning using for the menu-item-iconic an image that was 24px x 24px.

I tracked this down to the modern menu.css doing this (in Bug 293207):

.menu-iconic-icon {
  min-width: 16px;
  min-height: 16px;
  list-style-image: inherit;
}

I propose porting toolkit Bug 363130 :(make menuitem icons 16x16px consistently on Windows/Linux, to match Mac.) and just use:

width: 16px;
height: 16px;
Comment 1 Philip Chee 2009-09-18 07:21:15 PDT
Created attachment 401436 [details] [diff] [review]
Patch v1.0 Use width/height instead of min-width/min-height.

Minimal patch. Backports a fix from tookit themes.
Comment 2 neil@parkwaycc.co.uk 2009-09-18 08:06:32 PDT
Comment on attachment 401436 [details] [diff] [review]
Patch v1.0 Use width/height instead of min-width/min-height.

We have some non-16x16px icons. The ones I can think of offhand are in the Image Properties dialog in Composer.
Comment 3 neil@parkwaycc.co.uk 2009-09-18 11:01:57 PDT
(In reply to comment #0)
> While working on the Lightning integration I noticed that one of the menu items
> was significantly taller than the others in Modern but not in the Default
> theme. This was due to lightning using for the menu-item-iconic an image that
> was 24px x 24px.
Surely that's suboptimal, if the image is getting scaled all the time?
Comment 4 Philip Chee 2009-09-18 18:59:31 PDT
> Surely that's suboptimal, if the image is getting scaled all the time?

This is even more inexplicable since there is the equivalent 16x16 graphic in mail-toolbar-small.png in the Thunderbird skin which lightning doesn't bother to use. Of course SeaMonkey doesn't have mail-toolbar{-small}.png, so all that happens is that a blank 24x24 area takes up vertical space in the menuitem.
Comment 5 Philip Chee 2009-09-18 19:00:12 PDT
Also see Bug 516178 (Lightning uses hard coded Thunderbird skin paths to images).
Comment 6 Philip Chee 2009-09-20 10:55:10 PDT
Created attachment 401724 [details] [diff] [review]
Patch v1.1 Additional fix for editorDialog.css

> We have some non-16x16px icons. The ones I can think of offhand are in the
> Image Properties dialog in Composer.

Added fix to editorDialog.css from classic.
Also removed some tabs.
Comment 7 neil@parkwaycc.co.uk 2010-06-01 07:20:28 PDT
Places where we currently force 16x16 menu icons become redundant, of course.
Comment 8 Philip Chee 2010-07-20 01:21:10 PDT
Created attachment 458593 [details] [diff] [review]
Patch v1.2 Remove redundant styles.

> Places where we currently force 16x16 menu icons become redundant, of course.
Fixed.

I looked at the tabs in editorDialog.css via CVS blame and they were there all the way back to 1.1 sspitzer :P .
Comment 9 Philip Chee 2010-07-20 08:36:41 PDT
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/62a9693b8d4b

Note You need to log in before you can comment on or make changes to this bug.