426930 - "Close" icon in File menu looks pixellated/gross on Linux

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Daniel Holbert [:dholbert]]

See attached screenshot -- the "Close" icon looks kinda gross in Firefox's file menu on Linux.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screenshot

Comment 2

[Daniel Holbert [:dholbert]]

I think the icon is just a zoomed-in version of the close-icon that appears directly on tabs.

This icon looks ok in its smaller form, directly on the tabs.  But when it's zoomed in, in the file menu, it looks kinda gross.

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

As part of uplifting gtk icons can we check to make sure they are coming in at the expected resolution to avoid these types of problems?

Comment 4

[Daniel Holbert [:dholbert]]

FWIW, the icon's also zoomed & gross in the context menu when I right-click on a tab.

Comment 5

[Michael Ventnor]

This is only the Human theme because this icon does not get sent at 16x16 but at a different size.

Not sure how we can fix this. We might be able to use max-height/width, but then that might regress some icons on other themes.

Comment 6

[Michael Monreal [:micmon]]

WTF... does the Human theme include a 16x16 gtk-close icon? If yes, how does it look in Nautilus for example? I would tend to call this a theme bug, but on the other hand, I cannot imagine that Ubuntu doesn't ship this: strange!

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to comment #6)
> If yes, how does it
> look in Nautilus for example?

Looks fine in nautilus -- looks exactly the same as it does on a Firefox tab, which is the correct size. (not zoomed)

Comment 8

[Michael Monreal [:micmon]]

Totally weird... can you please test another icon theme (GNOME) and see if that makes any difference? The close icon has always looked normal for me, both in menu and tabs.

Comment 9

[Michael Ventnor]

We force icons in menus to 16x16 (to prevent any icons that may be bigger) but there is no such height or width rules on the tab close button.

Another problem is that I've seen screenshots of themes where menu gtk-close is much bigger than normal, almost button size. So either Ubuntu will have to comply and change their close icon to 16x16, or we'll have to figure out a good way to force images down if they're too big (but not up if they're too small). max-width and max-height don't exactly do what we want, I think. They completely screw up width/height ratios if given a big image whose width != height.

Comment 10

[Daniel Holbert [:dholbert]]

(In reply to comment #8)
> Totally weird... can you please test another icon theme (GNOME) and see if that
> makes any difference? The close icon has always looked normal for me, both in
> menu and tabs.
> 

I tested all of my available themes, on Ubuntu 7.10:
  ClearLooks:  Fine
  Crux:        Fine
  Glider:      Fine
  Glossy:      Fine
  High Contrast Inverse:             Fine
  High Contrast Large Print Inverse: Fine
  Human:       **BROKEN**
  Mist:        Fine

Note that I only saw 3 distinct close icons in these themes -- one for Human, one for the High Contrast themes, and a third one for the rest of the themes.

Comment 11

[Daniel Holbert [:dholbert]]

FWIW, I still see this, with updated OS & browser.

OS:       Ubuntu 8.10 (using default "Human" theme)
Browser:  Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081114 Minefield/3.1b2pre

Comment 12

[Daniel Holbert [:dholbert]]

Attachment: screenshot: shows both scaled & unscaled icons

Comment 13

[Andreas Nilsson (:andreasn)]

I think this might be due that human-icon-theme ship gtk-close in 8x8 (and this is getting used in the tab) as well as 16x16 (that should be used in the menus, but apparently don't in this case). gnome-icon-theme only ships a 16x16 for gtk-close.

Comment 14

[Alex Faaborg [:faaborg] (Firefox UX)]

This is a slightly meta comment, but what is the standard for including menu icons?  On other platforms they are only supposed to be used on a few of the high frequency menu items, to make them easier to spot.

Comment 15

[Andreas Nilsson (:andreasn)]

Alex: There isn't really any kind of agreement on that. For most apps it seems that the application developer tend to use icons for as many items as possible, unless the artist tell him that he really don't have time to draw those.
The idea we had for Firefox (and we implemented that in a couple of other apps as well) is that if a icon exists in a toolbar, it can also exist in the menu (to make the visual connection).
There is currently a bug open in GNOME bugzilla about our current icon overusage with mpt's suggested guidelines: http://bugzilla.gnome.org/show_bug.cgi?id=557469

Comment 16

[Alex Faaborg [:faaborg] (Firefox UX)]

Ok, good background information to have.  Even without formal guidance we might want to consider scaling back the use of menu icons, until a standard is set for us to follow. Filed bug 465669.

Comment 17

[Daniel Holbert [:dholbert]]

FWIW, this bug affects the Thunderbird 3.0 nightlies, as well. (no big surprise)

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20081223 Shredder/3.0b2pre

Comment 18

[Dão Gottwald [:dao]]

Attachment: remove the icon from the File menu and the tab context menu

Comment 19

[Michael Ventnor]

I don't like this. The close icon in the menu is definitely a standard (even if de facto) and it's only the Human theme that doesn't follow the 16x16 icon. Other themes (especially the more popular Clearlooks) shouldn't have to lose out because of Human.

I wish there was a way to make icons appear at their original size (scaling back to 16x16 if they're too big). There probably is and I'm just not fluent in CSS, though... :)

Comment 20

[Dão Gottwald [:dao]]

Attachment: use the 24px icon

We can use the 24px icon and scale it back to 16px.

Comment 21

[Dão Gottwald [:dao]]

Note that bug 422179 is the reason why Human's tiny icon looks extra ugly when we scale it up.

Comment 22

[Michael Ventnor]

Comment on attachment 375777 [details] [diff] [review]
use the 24px icon

I like this better. Damn Human theme.

r=me if you change to size=button which is slightly smaller, and instead of citing the bug number mention the fact that not all themes provide the close button at 16x16 for the menu size (e.g. Human).

Comment 23

[Dão Gottwald [:dao]]

I adhered to size=toolbar, because 24px look sharper than 22px when scaled down to 16px.

http://hg.mozilla.org/mozilla-central/rev/1d8e7e3c0c60

Comment 24

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ff06e7e1ef32

Comment 25

[Daniel Holbert [:dholbert]]

A quick MXR search for search for "gtk-close?size=menu" turned up a few other places that need this: the view-source window's "File" menu and the Places Organizer's "Organize" menu:
http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/gnomestripe/mozapps/viewsource/viewsource.css#56
http://mxr.mozilla.org/mozilla-central/source/browser/themes/gnomestripe/browser/places/organizer.css#200

Both of those menus have broken "close" icons right now.

Could we fix both of those in a follow-up patch?

Comment 26

[Dão Gottwald [:dao]]

Attachment: same fix for View Source and the library

Yes, we missed these menus. Thanks, Daniel.

Comment 27

[Andreas Nilsson (:andreasn)]

using a 24x24 image and scaling it down to 16x16 still makes it looks terrible, just in another way. I would rather loose the icon in the menu in that case.
I still feel this is a bug in the human-icon-theme, and not Firefox.

Comment 28

[Dão Gottwald [:dao]]

(In reply to comment #27)
> using a 24x24 image and scaling it down to 16x16 still makes it looks terrible,
> just in another way.

Do you have any evidence for that, or was it just a guess?

Comment 29

[Daniel Holbert [:dholbert]]

The scaled-down-24x24 image looks fine to me, in a debug build I made earlier today, FWIW.

Comment 30

[Michael Monreal [:micmon]]

I second Andreas, scaling 24x24 down to 16x16 will look bad in most cases. Anyway it makes things worse as the only problem is Ubuntu shipping a non-standard icon size in their default theme.

Also, don't make the horrible assumption that size=toolbar is always 24px. It's not. Themes that want to minimize screen use set this to 16px. GNOME's large a11y theme uses 32px for this.

Comment 31

[Daniel Holbert [:dholbert]]

Attachment: demo of old behavior vs new behavior

Michael / Andreas, just so we're on the same page, here's a screenshot to show you what the new downscaled icon looks like, compared to the old upscaled version.

I agree that scaling 24px (or 32px) down to 16px isn't ideal.  However, it looks **much** better than 12px scaled up to 16px, and it looks great in this case. :)

If you want a better long-term solution (or if change this happens to break behavior on other themes in a demonstrably bad way), feel free to file another bug on that.

Comment 32

[Michael Ventnor]

Yes, but your theme is Human. There are more themes than Human out there (and more popular, too). We shouldn't be accommodating single themes no matter how popular. If anything I've learned (from the libnotify bug) it's a terrible long-term solution to accommodate hacks based on one (theme|distro|version etc).

If anything, I agree the Human theme should change. We've affected all of Linux Firefox builds just for Ubuntu Human users, if anything it's Human's fault for deviating. I'm starting to regret my r+ on this and really don't want this hack in the tree anymore.

Comment 33

[Michael Ventnor]

Isn't there a way to constrain the images to 16x16 but keep them at the original size if smaller, which is what GTK does? If max-width and max-height don't work, is there any other CSS trickery we could use?

Comment 34

[Dão Gottwald [:dao]]

I've of course tested this this with themes other than Human. That's why I've chosen "toolbar" rather than "button" (with Human, both are 24px anyway; with other themes, "button" is 22px). The latest concerns don't seem to be based on actual testing. I also don't see how doing the same thing with a 32px icon would be any worse -- in fact that seems to be *better* suited for being scaled down to 16px.

I haven't questioned the fact that this is a bug in the Human theme. I want to improve Firefox' look anyway. Hence the XXX comments in the patches.

(In reply to comment #33)
> Isn't there a way to constrain the images to 16x16 but keep them at the
> original size if smaller, which is what GTK does? If max-width and max-height
> don't work, is there any other CSS trickery we could use?

Not without XUL changes, AFAIK.

Comment 35

[Alex Faaborg [:faaborg] (Firefox UX)]

Does anyone have contact information for the people working on the Human theme?

Comment 36

[Andreas Nilsson (:andreasn)]

The maintainer of the Human icon theme is Kenneth Wimer at Canonical. I've spoken to him and he have confirmed the issue.
Filed as https://bugs.launchpad.net/human-icon-theme/+bug/385903

Comment 37

[Kenneth Wimer]

Actually, this is not fixed at all :) It seems that firefox does the icon lookup incorrectly and for some reason uses the 12x12 icon and scales it up. When I move the 8x8 and 12x12 directory listings to the end of the list it finds the 16x16 icon and uses it correctly. This work-around however makes all apps which look correctly for 8x8 or 12x12 icons use a larger version (ie nautilus) :(