511785 - Restyle the Places window toolbar

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Markus Stange [:mstange]]

Attachment: v1

This patch is on top of bug 508940.

This patch does the following:
 - Get rid of the toolbar button background images and replace them with
   borders, gradients and box shadows.
 - Combine the toolbar button glyphs into one file, places/toolbar.png.
 - Share some of the round rect button stuff in shared.inc.
 - Fix some things for the console Clear button (text alignment and wrong
   inactive gradient).

For the pressed state, I had to use a little trick in order to get around bug 466572.
Dão, when you review this, please also review the new image. I don't know how to deal with color management and gAMA chunks, or how to tell whether they're included in that image.

Comment 1

[Markus Stange [:mstange]]

Attachment: v2

use -moz-background-origin: border so that the left and right borders are subject to the same color mixing as the top and bottom borders

Comment 2

[Markus Stange [:mstange]]

Attachment: v2.1

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 395768 [details] [diff] [review]
v2.1

>+#placesToolbar > toolbarbutton {
>+  list-style-image: url("chrome://browser/skin/places/toolbar.png");
>+  margin: 4px 6px 5px;
>+  padding: 1px 5px;
>+  -moz-border-radius: 100%;  
>+  border: 1px solid @roundRectButtonBorderColor@;
>+  -moz-box-shadow: @loweredShadow@;
>+  background: @roundRectButtonBackground@;
>+  -moz-background-origin: border;
> }
> 
>+#placesToolbar > toolbarbutton:not([disabled="true"]):active:hover,
>+#placesToolbar > toolbarbutton[open="true"] {
>+  background: rgba(0, 0, 0, 0.6);
>+  border-color: transparent;
>+  -moz-box-shadow: @roundRectButtonPressedInnerShadow@,
>+                   @loweredShadow@,
>+                   inset 0 0 0 20px @roundRectButtonPressedBackgroundColor@;
> }

Can you rename roundRectButtonPressedBackgroundColor to roundRectButtonPressedBackground and actually use it as background, as elsewhere? rgba(0, 0, 0, 0.6) could be used as an inset shadow if needed, as far as I can see.

Also, for things that are used for generic toolbar buttons as well as "roundRect" buttons, maybe do s/roundRect/toolbarbutton/?

Comment 4

[Markus Stange [:mstange]]

(In reply to comment #3)
> Can you rename roundRectButtonPressedBackgroundColor to
> roundRectButtonPressedBackground and actually use it as background, as
> elsewhere? rgba(0, 0, 0, 0.6) could be used as an inset shadow if needed, as
> far as I can see.

That wouldn't achieve the same thing, would it? rgba(0, 0, 0, 0.6) needs to blend with the toolbar background and must not be influenced by roundRectButtonPressedBackgroundColor.

> Also, for things that are used for generic toolbar buttons as well as
> "roundRect" buttons, maybe do s/roundRect/toolbarbutton/?

toolbarbuttonButton? ;-P
Sure, I can do that.

Comment 5

[Markus Stange [:mstange]]

Attachment: v3

Same patch with s/roundRectButton/toolbarbutton/

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 402226 [details] [diff] [review]
v3

>+%define toolbarbuttonCornerRadius 3px

I think you actually want something like roundRect here, since the toolbar buttons in the main window have a different radius.

Comment 7

[Markus Stange [:mstange]]

I think I'll leave it toolbarbuttonCornerRadius, since it's the radius that toolbarbuttons are supposed to have. We only use the wrong radius in the Places window because we want to be consistent with the main browser window, which uses the wrong radius in order to look less similar to Safari.

Comment 8

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/50adf6bd6ed6

Comment 9

[Markus Stange [:mstange]]

Comment on attachment 402226 [details] [diff] [review]
v3

Actually, we don't really need this on 1.9.2, and I'd have to merge around bug 508940...

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 402226 [details] [diff] [review]
v3

a192=beltzner, more last minute theme polish, what could go wrong? :)