9.78 KB, image/png
1.41 KB, patch
|Details | Diff | Splinter Review|
1.83 KB, patch
|Details | Diff | Splinter Review|
3.52 KB, image/png
See attached screenshot -- the "Close" icon looks kinda gross in Firefox's file menu on Linux.
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.
Component: Menus → Theme
QA Contact: menus → theme
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?
FWIW, the icon's also zoomed & gross in the context menu when I right-click on a tab.
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.
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!
(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)
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.
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.
(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.
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
Created attachment 348238 [details] screenshot: shows both scaled & unscaled icons
Attachment #313514 - Attachment is obsolete: true
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.
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.
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
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.
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
Created attachment 375620 [details] [diff] [review] remove the icon from the File menu and the tab context menu
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... :)
Created attachment 375777 [details] [diff] [review] use the 24px icon We can use the 24px icon and scale it back to 16px.
Note that bug 422179 is the reason why Human's tiny icon looks extra ugly when we scale it up.
Depends on: 422179
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).
Attachment #375777 - Flags: review?(ventnor.bugzilla) → review+
I adhered to size=toolbar, because 24px look sharper than 22px when scaled down to 16px. http://hg.mozilla.org/mozilla-central/rev/1d8e7e3c0c60
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.6a1
Keywords: checkin-needed → fixed1.9.1
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?
Created attachment 375862 [details] [diff] [review] same fix for View Source and the library Yes, we missed these menus. Thanks, Daniel.
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.
(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?
The scaled-down-24x24 image looks fine to me, in a debug build I made earlier today, FWIW.
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.
Created attachment 375912 [details] 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.
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.
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?
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.
Does anyone have contact information for the people working on the Human theme?
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
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) :(
You need to log in before you can comment on or make changes to this bug.