the original plan was to be more unified-like, and we seem to have lost that a little. We can tweak this by reusing the bookmark toolbar CSS, and cleaning that up a little, patch upcoming for late inclusion, already got signoff from mscott since this affects Thunderbird as well.

Comment on attachment 240426 [details] [diff] [review] fix up toolbars Looks good here. r=beltzner

fix a couple nits, let's land this and be done with it

Comment on attachment 240429 [details] [diff] [review] clean up a bit more a=beltzner for drivers

I actually liked it white! It looked a great deal better then the light gray.

(In reply to comment #3) > Created an attachment (id=240429) [edit] This looks wrong to me: >+ background-image: url("chrome://global/skin/toolbar/toolbar-background.gif") repeat-x;

(In reply to comment #7) > Created an attachment (id=247597) [edit] > trunk patch Sorry, I have to ask again: Why repeat-x as a value for background-image? Is this a Mozilla quirk?

Dão: you'll have better luck if you directly state your objection in detail, rather than making vague statements like "this looks wrong." If instead you say: Mano: if you're going to break up a "background" shorthand rule into separate rules, I think you should put the repeat into "background-repeat" where the specification says it belongs, rather than tacking it onto the end of the "background-image" rule, since even if that does happen to work in XUL, there's no reason why it should, nor reason to expect it to keep working. then you're more likely to get a reaction.

the graphic used is basically adding a groove to the top of the toolbar, with normal repeat that would show in the wrong place, unless the image was super-tall. repeat-x is a w3c standard, but I misused it in the original patch.

What's that you say? Throw more words at this? Nobody's saying it isn't a standard, what they are saying is that with our previous, and proposed future, use of "background-image: ... repeat-x" we are either doing absolutely nothing, no repeat and no image, if our invalid rule is properly being dropped on the floor, or we are exploiting a bug in our parsing of CSS (which I don't think we are, I think we're doing nothing), because while "background: ... repeat-x" is valid, if we use individual rules rather than the single "background:" shorthand rule, we need to instead use "background-image: url(...); background-repeat: repeat-x;" So, rather than explaining the intent, didn't you mean "r-, please fix my mistake and request branch approval for a fix there, too"?

Ah, no wonder it seems to work in 2.0: just to keep people on their toes, attachment 240429 [details] [diff] [review] wasn't actually what you checked in to the branch - sometime between when you attached it and when you checked it in, you realized that you couldn't tack a repeat-x on at the end of a background-image, so you fixed what you checked in, but not the patch that Mano's now ported to the trunk. I was starting to really wonder wtf, until I realized that (and that the image isn't actually applied to the navigation toolbar).

mozilla/browser/themes/pinstripe/browser/browser.css 1.37 mozilla/toolkit/themes/pinstripe/global/jar.mn 1.25 mozilla/toolkit/themes/pinstripe/global/toolbar.css 1.7 mozilla/toolkit/themes/pinstripe/global/toolbar/toolbar-background.gif 1.2

Looks like this broke the address bar on the 120706 trunk builds? Will include image.

(In reply to comment #15) > Created an attachment (id=247894) [edit] > Broken address bar I'd say that's unrelated, since the patch touches background rules only.

I filed bug 363125 for that broken location bar