User-Agent: Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040120 Firebird/0.7+ I like using the default firebird theme in its 'full' state, with icons and text. Another caveat: I'm using "Large Fonts" on Windows2k. So my fonts probably take up some more pixels than the default screen fonts. This is a very common choice, especially for laptops with small screens and very high resolution. What I notice is that because the word "Forward" is longer than any other word, the "Forward" button is bigger (wider) than any other button. For similar reasons, the "Back" button is smaller than any other button. The length of the word on the button doesn't seem like a good way to determine their size, and it makes the UI look a little messy IMHO that all the buttons have a different width based on the word length. Also, the "Back" button to me is one of the most important buttons I use, and it shouldn't be the smallest from a functional point of view. I would propose adding "min-width" tags to the default theme for the buttons. I did this myself, adding "min-width: 50px;" to skin\classic\browser\browser.css for the blocks ".toolbarbutton-1" and ".toolbarbutton-menubutton-stack" blocks at lines 150 and 162. This makes icons+text look great to me, but it has the side-effect of keeping the min-width even when you are viewing icons-only, which I realize is a waste of space and undesirable. I'm sure a more experienced themer could get the same result, but make no minimum width for icons-only. I'm not sure how font size is determined in the theme, but if it is variable based on the OS screen fonts, I suppose that a min-width based on pixels might be problematic. If there was some way to base it on the width of the word "Forward" that would be ideal. Reproducible: Always Steps to Reproduce: 1. Use default theme 2. Choose "icons+text" from "Customize..." for the theme 3. (maybe) use "Large Screen Fonts" in Windows2k Actual Results: All major navigation toolbar buttons have different widths, based on the length of their word label. Expected Results: Standardized widths of navigation buttons.

-> arvid, for consideration

I should mention that I'm also using the small icons.

Confirming for consideration

Sorry this isn't a patch, I'm still a little new. browser.css which correctly implements min-width only when text is showing on toolbar buttons.

I attached a version of browser.css (from \chrome\classic\skin\classic\browser) which implements a minimum width only if text is showing on the toolbar buttons. It uses 3.7em, which will scale with screen font, and is not quite as long as 'Forward' word length, but slightly longer than 'Reload' word length, so most buttons except 'Forward' have a uniform width. For 'icons' mode with no text showing, behavior should be the same as it used to be.

Sorry, it appears that for some reason depressed buttons in the attachment from comment #4 always have the minimum width, even when they are icons-only. I guess it's not quite right.

pretty much the same approach as Matt's attempt, but used the width of the "New Window" button as a reference, and added a tweak to make menu-buttons (back and forward) work properly.

(In reply to comment #7) > Created an attachment (id=141984) > patch making toolbar buttons the same width in icons and text mode hmm. seems to be some slight rendering weirdness from this patch, in combination with the patch for dropmarker hover states (bug 226274 comment #4). probably pixel rounding errors (bug 63336). sometimes the edge between menu buttons (forward, back, etc) and their dropmarkers either gets an extra pixel or looses a pixel. probably due to the use of "em" units in this patch. not sure if there's currently any way around it.

update of patch from comment #9. makes all toolbar buttons, including the go button, a uniform width based on the button with the widest label: "New Window." basically like IE's "Show text labels" mode. now uses 'ex' values that round to even pixels as outlined in: http://kb.mozillazine.org/index.phtml?title=Dev_:_Theme_Development_:_em_vs_ex unlike the previous patches, this prevents pixel rounding errors on Windows systems using default font sizes (which is probably the majority of users), and probably most other combinations of OSs and font sizes.

the patch in comment #10 only affects buttons in the full "Icons and Text" mode. this alternate patch includes a uniform button width for toolbar buttons in "Text" only mode. it also uses the same evenly rounding 'ex' unit technique as that patch, for compatibility with the dropmarker hover states patch in bug 226274 comment #4.

Awesome work on the theme! Now, please pardon me for bringing up an annoying detail: :) Should the minimum width be based on the "New Window" button? This is a long text string, and most people don't have it visible on their toolbar anyway. I'm wondering if it would be ok for a few long buttons to be wider than the other buttons, while ensuring that MOST buttons are of uniform width.

i don't know. that's a decision that might need to be made by someone "higher up." i chose the "New Window" button, because it it the biggest, and therefore would cause all buttons to have the same minimum width. this ensures a consistent, uniform appearance. but i understand your point - it is a bit wide for many buttons. but what width to use if not this one? you might want to try to get someone that makes judgements about the look and feel of the ui to check out this bug. the patch will be simple to update, we just need to figure out what to change it to, if it needs to be changed at all.

I use 6.8ex for my minimum widths: it's pretty tight, but still keeps buttons like 'Back' and 'Stop' from turning into slivers.

Bug 60553 proposes a true under-the-hood fix.

bug 60553 is basically calling for what IE does which is to dynamically size the buttons to all be the same size and the width of the widest button. I think that short of that (and we're probably not going to get that for 1.0) we need something that gets most of the buttons to a uniform size. Bookmarks and New Window can be a bit fatter and folks probably won't notice or mind. Right now, I think if we can find a hack that doesn't make the buttons too wide for the common case (the default toolbar configuration) while still accomodating customization, that's what we should go with. Can one of you on windows XP (and in "classic" mode or on win2K if possible) post screenshots of the latest patch with the default toolbar config and with a "full" toolbar that contains all of the icons available in the customize pallet? That would be a big help.

Having all the buttons the same size (like IE) would make the toolbar look more natural.

*** Bug 236961 has been marked as a duplicate of this bug. ***

Not a "blocker"

An up-to-date patch. This patch only affects icons+text mode; no changes are made to text-only mode or the default icons-only mode. Min-width is set to 9.5ex for most toolbar buttons. This makes the buttons look just wider than square with the default Windows font (8pt Tahoma) and is wide enough to cover everything in the browser except Downloads and Bookmarks, which are *very* slightly wider, and New Window, which is noticably wider. The Go button is exempt since it's not a normal toolbar button, but I added some padding on the right to make it seem less squashed up. Almost all icons in the bookmarks manager are wider than 9.5ex anyway, but at least the change does add some (IMO necessary) padding to Move and Delete. There's no check for icons+text mode here, since none of the other style rules seem to have it and I couldn't find any way to select another mode in the bookmarks manager. The back and forward inner buttons have a slightly smaller min-width of 9ex, since the additional dropmarker sections make them appear wider than the other buttons anyway. I think the effect balances out well. I also took the liberty of copying over the toolbarbutton margin and padding settings from browser.css to the other two files for consistency. In particular, the margin-right: 0 fix for toolbarbutton icons is pretty much a necessity for icons+text mode, otherwise the icons look quite badly misaligned. Who would be a good person to ask for review?

The full set of browser buttons with this patch applied, on an IE-like toolbar configuration, under both Luna and Classic.

I didn't take a screenshot in the default icons-only mode since that mode is unaffected.

(In reply to comment #21) > Created an attachment (id=161503) > Patch for browser, bookmarks manager and help Looks pretty good. But something needs to be done for text only mode as well. And it might be good to leave Go Button tweaks to bug 235277. Some nitpicks: Native inner menubuttons are the same width as normal ones - not including the dropmarker, so in the context of this patch, they would be 9.5ex. I assume you made them 9ex to allow room for the dropmarkers, but in that case, why not just leave the .toolbarbutton-menubutton-buttons alone, so the outer toolbarbuttons are 9.5ex like all the rest? For uniformity, either the inner menubutton, or the whole outer button including the dropmarker should be the same as the rest. At the current value, neither is the case. Is there some place more upstream that we could define this width so that it doesn't have to be repeated for each app? global/toolbarbutton.css or perhaps global/browser.css. Hmm, lemme play with this a little, i'm getting a few ideas...

This patch does it all from a single selector in global/toolbarbutton.css. -It's defined globably, so it filters down to browser, bookmark manager, help, etc. -Instead of setting a minimum outer-button width, it sets the width of the label which sizes the button from the inside. Regular toolbarbuttons and inner .toolbarbutton-menubutton-buttons are the same width. -It works for both Icons+Text and Text Only (though that could easily be changed with a seperate rule using a different width for Text Only). -It only affects children of toolbars, so, for example, toolbar bookmarks and tab close aren't affected. -It only affects toolbarbuttons with a "label" attribute. -It doesn't affect Icons Only mode, since labels are display: none'd. In my quick testing, it seems to play nice with the standard kit. Extensions and such might be a different story. And it was a late-night quickie, so there may be something obvious i'm overlooking. Might not be the way we wanna go with this, but it's an option to consider.

(In reply to comment #25) > Created an attachment (id=161553) > patch: different approach, same effect A side-effect - it doesn't hold during Customize, so the buttons shrink back to their "real" size. The selector breaks because toolbarbuttons are put in a palette wrapper during customize. They return to the fixed size after exiting customize. Probably fixable with more CSS selector tom-foolery.

(In reply to comment #26) > A side-effect - it doesn't hold during Customize, so the buttons shrink back to > their "real" size. The selector breaks because toolbarbuttons are put in a > palette wrapper during customize. They return to the fixed size after exiting > customize. Probably fixable with more CSS selector tom-foolery. You can get around that by using the descendant selector instead of the child selector between the toolbar and toolbarbutton, or adding another identical rule with an extra "> toolbarpaletteitem" before the toolbarbutton. (If you use the former approach, you can then simplify the second selector to a child instead of a descendant). I like the idea of just changing toolbarbutton.css better, but I couldn't actually get the method you used to work on normal toolbarbuttons on Luna - DOMi reports that the toolbarbutton text does indeed have the correct min-width of 9ex, but its computed width was still the same value as you get from shrinkwrapping the button. What it DID affect was the find toolbar, and it did so most strangely - hovering the buttons showed that the icon was actually pushed to the left, slightly outside of the hover area. Switching to Windows Classic, this does actually seem to work - it does improperly widen the find toolbar buttons still, but the toolbar buttons get their minimum width and the find toolbar icons are within the hover area. Perhaps a bug in Windows native theming. The toolbar buttons look wider than with my patch, too - presumably because there's padding etc. on elements containing the text - so perhaps a smaller value would be wise if the text is going to have min-width rather than the button. I think if this global approach is going to be used, it does need to check that the toolbar is [mode="full"] or [mode="text"] and probably set the width on the button rather than the text, since giving the text min-width seems buggy at best. Going back to using .toolbarbutton-1 and .toolbarbutton-menu-button would probably work. I'd be happy to just make another version of my own patch with 9.5ex for .toolbarbutton-menubutton-button if that's generally preferred. I realise the standard is to make the inner button the same width as other buttons, but the back button just seemed a bit too fat that way. Perhaps it wouldn't on Windows Classic where there's no visible hover state on the dropmarker (though that's a bug in itself). As for just setting the outer button min-width and ignoring .toolbarbutton-menubutton-button, I found that this caused the inner button to shrinkwrap and, hence, the back button ended up with a much larger dropmarker (to fill the remaining space) than the forward button, which looks very strange indeed (again probably less so on Windows Classic, with its lack of dropmarker hover).

Forgot to attach this yesterday. This is my test version of Winstripe, modified into a separately installable theme so changes are easily comparable with the original. This includes the patch from attachment 161503 [details] [diff] [review], and is probably the fastest way to test the changes out - just save it and drag it onto the themes window.

(In reply to comment #27) > Going back to using .toolbarbutton-1 and .toolbarbutton-menu-button would > probably work. That's exactly why it won't work - the Bookmark Manager buttons won't be covered by that. And making a seperate rules just for BM kinda defeats the purpose of this way of doing it. I'll keep tweaking on it, and see if i can get something more solid. I've already worked around the customize problem, but there's still a few more things to straighten out. I did overlook the Find Toolbar on the first try, and i'm not easily able to test on XP/Luna either. If it's not gonna work out, we'll just go with the regular methods we started with.

This uses the same method as my previous patch, since I wasn't personally able to find a way to make the combined method suggested by miahz work. It fixes a few problems with the original: + Should actually apply cleanly + The dropmarkers on the back and forward buttons in Help were being pushed to the right, because margin-right: 2px was incorrectly applied to the inner button. (This is how it's applied in browser.css, but it's later overriden by an identical selector.) + Small icons + text mode wasn't affected. Fixed by removing a redundant second definition of min-width: 0px for small icons mode (it's defined above for all modes, and then overriden for icons+text) + I caved in and used 9.5ex for inner buttons as well as everything else. There's only a 3px size growth compared to my original screenshots, so they're still pretty valid for comparison purposes. I didn't include text-only mode since, after testing it a bit, I personally feel it's better as it is. Essentially, I think these are the reasons min-width is needed for icons+text: a) Toolbar icons that are taller than they are wide look strange and have an uncomfortable hit area b) Icon spacing is visibly quite uneven otherwise Neither of these really apply to text-only mode. The toolbar buttons in text-only mode seem more akin to menubar buttons, which certainly don't have a minimum width, and IMO would feel pretty weird if they did. And after all, the initial description only really makes mention of icons+text mode.

Updated installable Winstripe for quick testing/comparison.

Neil, I'm looking at your latest patch. It looks good, except for the Go button which grows in size when you click on it and pushes the URLbar to the right. Please fix that and I'd be glad to land this tonight. Anyone have a comment on Neil's patch?

One little thing I noticed is that whatever button is immediately to the left of the URLbar has one pixel shaved off when you mouse over it. See screen shot. Any ideas? The Go Button problem is unrelated to the patch. But please incorporate this change: #go-button { padding: 2px 3px; } #go-button:hover:active { padding: 3px 2px 1px 4px; }

I haven't been able to reproduce the 1px cutoff issue so far on Luna or Classic. Perhaps it's triggered by some specific aspect of that toolbar configuration. Does it happen with the default-like config from attachment 161506 [details]? If not, a screenshot of the whole toolbar config might help me investigate. If so, I'm rather stumped. Is it definitely a consequence of this patch? What happens if you put a button like New Window there, which is wider than the given min-width anyway? If that's still cut off, I just have no idea what added rule could be causing it. I played with the Go button a little, and in the spirit of the patch unsquishifying the labels, I'd like to suggest following alternative defs: #go-button { padding: 2px 5px 2px 3px; } #go-button:hover:active { padding: 3px 4px 1px 4px; } i.e. an extra 2px on the right. I've verified that this does still prevent the button from growing in its pressed state; it also looks more balanced, IMO, since the button already has some padding/margin on its left but the text label hasn't got any noticable amount on its right. If you'd still rather stick with the definitions you gave before, though, let me know and I'll include those in the next patch version instead.

Strange. I can reproduce the clipped icon border effect .. only on certain icons like New Tab, Home, Cut, Copy, Stop regardless of where they are in the toolbar. I did this with a fresh profile. However I was not able to reproduce it on another machine. I'll do a test on another machine when I get home in an hour or two. Let's go with your padding values.

I wonder if this could have anything to do with ex units and rounding errors at certain font sizes. It seems an odd thing to be caused by it, but perhaps next time you are (or anyone else here who tests and happens to experience this is) on a machine/profile where the problem is occuring, it might be worth changing the min-width to a pixel value just to see if it helps matters.

(In reply to comment #37) > I wonder if this could have anything to do with ex units and rounding errors at > certain font sizes. I bet that's exactly what it is. I'll take a look at your newest patch and see i can tweak these last few things out.

Same as previous, except: + Made against current file revisions + Go button fixed + Don't add margin-right: 2px to full/outer buttons in help/bookmarks, per Stephen Horlander's recent checkin that removed the def from browser.css. Still unable to reproduce the clipping issue here - miahz, this patch is a better starting point to work from if you want to take a look. The line endings seem strange in the current browser.css (and in some recent PNG checkins, which I ended up hex editing to fix locally, but that's another issue). If the browser part of the patch looks strange, that's why.

We only have a few hours until the tree is closed for RC1. Miazh, are you seeing the same clipping that I am?

If you want a stopgap for RC1, we could use 57px for now, which is the computed value of 9.5ex at Windows' default 8pt font size. It's just as effective as 9.5ex for the default case, and while it's not as useful for larger font sizes, it doesn't make matters any worse. That's assuming that ex units are indeed the problem - has anyone been able to confirm that a px value solves it? I've been trying really hard to reproduce it here so that I can test, but I guess I'm just really (un)lucky.

I reproduced the clipping on my home machine. It wasn't apparent in the default toolbar but I saw it I put a few other buttons on there. I put this in my useChrome.css and it doesn't seem to be clipping. toolbar[mode="full"] .toolbarbutton-1, toolbar[mode="full"] .toolbarbutton-menubutton-button { min-width: 57px !important; } Oh and about the line endings in browser.css, I think I screwed them up when I did a check in last night :/

I suspect this is the best we can get before RC1.

checked in on branch! thanks Neil!

Sorry guys - didn't know we were looking at a tight deadline. And though i couldn't reproduce the visual rounding glitch, i can confirm that the computed widths were fractional at non-8-sized fonts. Using whole 9 or 10ex widths instead of 9.5ex seems to circumvent this in all the font sizes i tried. We'll get that in the next round i guess.

I wondered if whole ex units might help, but I didn't want to risk it with only a few hours to test, in case it caused problems in some scenario we couldn't catch. Definitely worth a look in the long term.

Fixed on trunk by the branch landing.

Text only mode was not fixed, if i'm not mistaken.