737934 - Adjust tab strip background for Aero glass as part of the Australis theme

Status: RESOLVED DUPLICATE of bug 738490

(Firefox :: Theme, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Tab Strip background

For Aero (Vista and 7), we need the glass behind the tabstrip to be more foggy so the title text of background tabs is still readable.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

The attached image is to show what it should look like. The actual implementation should use a
> background-color of hsla(210,23%,74%,.8)
and then on top of that background there should be a 
> box-shadow: 0 0 30px 25px rgb(174, 189, 204);

Comment 2

[Pranav Ravichandran [:pranavrc]]

Attachment: tabstrip background-color and box-shadow

The Australis theme is not available for nightly, so I checked this with the Aurora Australis theme

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 608399 [details] [diff] [review]
tabstrip background-color and box-shadow

Can you attach a screenshot?

All the changes should happen in browser/themes/winstripe/browser-aero.css and you would need to change some of the current styles there that already set a background-color.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Pranav Ravichandran [:pranavrc] from comment #2)
> The Australis theme is not available for nightly, so I checked this with the
> Aurora Australis theme

I just saw this now. The Australis theme found on AMO is not related to the theme we are implementing that will ship with all Firefox installs. All changes should be built upon the code in Nightly.

Comment 5

[Pranav Ravichandran [:pranavrc]]

Attachment: tabstrip in aero and non-aero themes

Just in case you still need it :)

Comment 6

[Girish Sharma [:Optimizer]]

I would like to take on this bug. The specs in the url [1] really don't talk about Tabs on Bottom mode. So I guess that the subtle background should only apply for tabs on top mode.
One more concern, when in the restored mode (not maximized) with tabs on top, the tab strip have extra top margin. The specs also do not talk about that.
Should the same background be applied to the tab strip starting from top of the window frame ? or only the tab strip ?

[1] http://people.mozilla.com/~shorlander/files/australis-designSpecs/australis-designSpecs-windows7-mainWindow.html

Comment 7

[Pranav Ravichandran [:pranavrc]]

Hey Girish. The problem here is that the box-shadow applied on the tabstrip with those values, overlaps the titlebar-buttonbox. The specs in the link don't talk about a box-shadow, but since it's mentioned here that a box-shadow should be used, it needs addressing. I sent Jared a mail about it, and I'm waiting for his reply.

About applying this only when tabs are on top, a change in the tabstrip's background color every time an user toggles that option would make the inconsistency very conspicuous IMO. Again IMO, applying the background-color to only the tab strip(and not the titlebar) in the restored mode wouldn't look very good either.

Comment 8

[Girish Sharma [:Optimizer]]

Hmm, I talked to Jared few hours ago, he said that he wanted to get this bug done and I wanted a bug to fix.
Yes I also feel the same about consistency, but the specs have very little details.

So the main points of concern are :
1. mode: tabs on bottom, should background of tab strip be changed ?
2. mode: restored, tabs on top: should the background continue from top of window frame to tab strip bottom ?

And as far as title bar is concerned, title bar should not be touched, only Tabstrip should be touched.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Sorry, usage of box-shadow isn't required, it was just one of my initial thoughts of how to fix this bug. Feel free to use whatever approach you find that works the best. After you get a patch put together we can see if the approach is acceptable, but don't feel constrained when trying out new ideas.

> So the main points of concern are :
> 1. mode: tabs on bottom, should background of tab strip be changed ?

I'm not sure. If you want you could try both and we could see what looks best.

> 2. mode: restored, tabs on top: should the background continue from top of
> window frame to tab strip bottom ?

Yeah, I think that's what we want.

> And as far as title bar is concerned, title bar should not be touched, only
> Tabstrip should be touched.

We might want to style both, but I'm not sure. I'm curious to see how it would look in either case.

Comment 10

[Pranav Ravichandran [:pranavrc]]

Attachment: proposed patch v2

Here's a patch with the following behavior:

1. Applies background-color only when tabs are on top.
2. In the restored mode, the titlebar and the tabstrip both have the same glossy appearance and the same background-color (without editing the titlebar, the color transition looks weird).
3. In the maximized mode, the tabstrip has the required background-color.

I will post screenshots.

Comment 11

[Pranav Ravichandran [:pranavrc]]

Attachment: Tab strip in restored mode

Here's the screenshot (with the same windows and wallpaper behind firefox)

Comment 12

[Girish Sharma [:Optimizer]]

(In reply to Pranav Ravichandran [:pranavrc] from comment #11)
> Created attachment 609836 [details]
> Tab strip in restored mode
> 
> Here's the screenshot (with the same windows and wallpaper behind firefox)

In restored mode, the rest of the 4px frame on left right and bottom of the window does not have any background, and the sudden change seems inconsistent.

Should the XUL frame of the window also have the background when tabs on top and restored mode.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 609821 [details] [diff] [review]
proposed patch v2

Review of attachment 609821 [details] [diff] [review]:
-----------------------------------------------------------------

The background-color doesn't blend in with the rest of the window frame. Can you try to fix that?

::: browser/themes/winstripe/browser-aero.css
@@ +112,5 @@
>      background: transparent;
>    }
>  
> +  #main-window[tabsontop=true]:not(:-moz-lwtheme) {
> +    background-color: hsla(210,23%,74%,.8) !important;

Why is "!important" needed here?

Comment 14

[Pranav Ravichandran [:pranavrc]]

This happens because the window's top-frame(titlebar) has been implemented in XUL over the frames that windows automatically draws. So #main-window selects that frame and the OS-drawn frames are left out. The left, right and bottom frames have not yet been implemented. This is the relevant bug: https://bugzilla.mozilla.org/show_bug.cgi?id=590945

Once the rest of the frames have been implemented, I think the patch would automatically change the colors of all the other frames.

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Pranav Ravichandran [:pranavrc] from comment #14)
> This happens because the window's top-frame(titlebar) has been implemented
> in XUL over the frames that windows automatically draws. So #main-window
> selects that frame and the OS-drawn frames are left out. The left, right and
> bottom frames have not yet been implemented. This is the relevant bug:
> https://bugzilla.mozilla.org/show_bug.cgi?id=590945
> 
> Once the rest of the frames have been implemented, I think the patch would
> automatically change the colors of all the other frames.

Fixing that bug will help, but there is likely a solution that exists today that can work-around the issue that bug 590945 is trying to solve.

Comment 16

[Dão Gottwald [:dao]]

The background isn't supposed to cover the whole window, so you shouldn't set it on main-window in the first place.

Comment 17

[Joshua M [:soapy]]

Attachment: WIP patch

Comment 18

[Joshua M [:soapy]]

Attachment: WIP patch normal window

Comment 19

[Joshua M [:soapy]]

Attachment: WIP patch maximized window

Comment 20

[Frank Yan (:fryn)]

Comment on attachment 619702 [details] [diff] [review]
WIP patch

Review of attachment 619702 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for writing a patch, Joshua!

The gradient in Stephen's mockups is made softer by extending into the region above #TabsToolbar. Is there something that prevents us from doing that? If Stephen actually prefers this patch's look, that's fine with me. If the mockup's appearance is preferred, I'd like to try to achieve it. Perhaps, setting the background on #navigator-toolbox would do it?

This looks good enough from a technical standpoint, so I'm handing this over to Dão. Please get ui-review from Stephen before landing this. I just want to make sure this is what we want visually, so we avoid a followup bug in which we redo this thing.

::: browser/themes/winstripe/browser-aero.css
@@ +216,5 @@
>      background-clip: padding-box;
>    }
>    #main-window[sizemode=normal] #TabsToolbar[tabsontop=true]:not(:-moz-lwtheme) {
>      margin-bottom: -1px;
> +    background-image: -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 26, 27, 0),

It looks like sometimes we include the full path and sometimes we don't. Dão, do you know why that is?

@@ +221,5 @@
> +                      -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 27, 27, 26),
> +                      -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 53, 27, 27) !important;
> +    background-repeat: no-repeat;
> +    background-size: 26px 27px, -moz-calc(100% - 52px) 27px, 26px 27px;
> +    background-position: 0 bottom, 26px bottom, 100% bottom;

Nit: `left bottom, 26px bottom, right bottom` is a little clearer, IMHO.

`26px` could be replaced with `center`, but then there might be rounding errors, so 26px is the safer choice, I think. :)

@@ +224,5 @@
> +    background-size: 26px 27px, -moz-calc(100% - 52px) 27px, 26px 27px;
> +    background-position: 0 bottom, 26px bottom, 100% bottom;
> +  }
> +  #main-window[tabsintitlebar]:not([inFullscreen]) #TabsToolbar[tabsontop=true]:not(:-moz-lwtheme) {
> +    background-image: -moz-linear-gradient(bottom, @toolbarShadowColor@ 1px, transparent 1px),

What does this do?

@@ +230,5 @@
> +                      -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 27, 27, 26),
> +                      -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 53, 27, 27);
> +    background-repeat: no-repeat;
> +    background-size: 100% 100%, 26px 27px, -moz-calc(100% - 52px) 27px, 26px 27px;
> +    background-position: center bottom, 0 bottom, 26px bottom, 100% bottom;

Same as above.

Comment 21

[Joshua M [:soapy]]

(In reply to Frank Yan (:fryn) from comment #20)
> Comment on attachment 619702 [details] [diff] [review]
> WIP patch
> 
> Review of attachment 619702 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for writing a patch, Joshua!
> 
> The gradient in Stephen's mockups is made softer by extending into the
> region above #TabsToolbar. Is there something that prevents us from doing
> that? If Stephen actually prefers this patch's look, that's fine with me. If
> the mockup's appearance is preferred, I'd like to try to achieve it.
> Perhaps, setting the background on #navigator-toolbox would do it?
>

#navigator-toolbox doesn't extend into the titlebar so it wouldn't be any different then skinning #TabsToolbar. I tried to do something I knew was possible until we are able to draw into the frame.

> This looks good enough from a technical standpoint, so I'm handing this over
> to Dão. Please get ui-review from Stephen before landing this. I just want
> to make sure this is what we want visually, so we avoid a followup bug in
> which we redo this thing.
> 
> ::: browser/themes/winstripe/browser-aero.css
> @@ +216,5 @@
> >      background-clip: padding-box;
> >    }
> >    #main-window[sizemode=normal] #TabsToolbar[tabsontop=true]:not(:-moz-lwtheme) {
> >      margin-bottom: -1px;
> > +    background-image: -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 26, 27, 0),
> 
> It looks like sometimes we include the full path and sometimes we don't.
> Dão, do you know why that is?
> 
> @@ +221,5 @@
> > +                      -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 27, 27, 26),
> > +                      -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 53, 27, 27) !important;
> > +    background-repeat: no-repeat;
> > +    background-size: 26px 27px, -moz-calc(100% - 52px) 27px, 26px 27px;
> > +    background-position: 0 bottom, 26px bottom, 100% bottom;
> 
> Nit: `left bottom, 26px bottom, right bottom` is a little clearer, IMHO.
> 
> `26px` could be replaced with `center`, but then there might be rounding
> errors, so 26px is the safer choice, I think. :)
> 
> @@ +224,5 @@
> > +    background-size: 26px 27px, -moz-calc(100% - 52px) 27px, 26px 27px;
> > +    background-position: 0 bottom, 26px bottom, 100% bottom;
> > +  }
> > +  #main-window[tabsintitlebar]:not([inFullscreen]) #TabsToolbar[tabsontop=true]:not(:-moz-lwtheme) {
> > +    background-image: -moz-linear-gradient(bottom, @toolbarShadowColor@ 1px, transparent 1px),
> 
> What does this do?
>
> @@ +230,5 @@
> > +                      -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 27, 27, 26),
> > +                      -moz-image-rect(url(tabbrowser/tabbar-glow.png), 0, 53, 27, 27);
> > +    background-repeat: no-repeat;
> > +    background-size: 100% 100%, 26px 27px, -moz-calc(100% - 52px) 27px, 26px 27px;
> > +    background-position: center bottom, 0 bottom, 26px bottom, 100% bottom;
> 
> Same as above.

When the window is maximized this makes up the "border" between the #TabsToolbar and the #nav-bar. I placed it here to override some code in browser.css. This was necessary because of the way this border is styled.

Comment 22

[Jared Wein [:jaws] (please needinfo? me)]

Once this patch gets r+, we'll probably delay landing it so that the patch for this bug and the patch for bug 738490 can land at the same time. Joshua, would you like to work on bug 738490?

Comment 23

[Dão Gottwald [:dao]]

(In reply to Jared Wein [:jaws] from comment #22)
> Once this patch gets r+, we'll probably delay landing it so that the patch
> for this bug and the patch for bug 738490 can land at the same time.

Not "probably". There's just no point in landing this separately. In fact, can this patch be moved to bug 738490?

Comment 24

[Jared Wein [:jaws] (please needinfo? me)]

I agree with Dão that we should merge this bug with bug 738490.

Comment 25

[Stephen Horlander [:shorlander]]

Comment on attachment 619703 [details]
WIP patch normal window

Clearing flags since this bug got duped.