858089 - Tab stroke polish

Status: RESOLVED FIXED

(Firefox :: Theme, enhancement)

Description

[Mike Conley (:mconley) (:⚙️)]

Attachment: WIP Patch (Windows only)

Across Windows, OSX and Linux GTK, the Australis curvy tabs spec calls for a 1px inner highlight on the stroke. We also aren't doing a very good job blending the stroke with the tabstrip border.

I'm attaching a POC patch shorlander just provided to me in IRC, Windows-only.

Comment 2

[Matthew N. [:MattN]]

I started looking into this and there are some edge cases such as when the nav-bar is hidden that make this is a little tricky.

Comment 3

[Matthew N. [:MattN]]

Stephen's WIP patch also handles the inner highlight and the overlap on the border shown in attachment 729570 [details].

Comment 4

[Matthew N. [:MattN]]

Attachment: WIP v.2 - Windows and OS X

I measured that the height of the tab above the navbar border (on Windows and OS X) is now 28px as specified by the spec. I haven't tested this on Linux yet.

Two ideas:
A) I considered margin-top: -1px on the content-box but I wonder if that will cause perf problems (e.g. a reflow) or will introduce bugs.
B) Use a 1px high fill that matches the bottom pixel color of the selected tab

Stephen: 
1) What were the tab-texture-*.png images for in browser-aero.css? Was it just that the colour should change in that case? Any reason why we can't just override the colours in the linear-gradient instead of using an image?
2) This works except for the case where all toolbars are hidden e.g [disablechrome] with the add-on manager. Stephen: Can we go ahead with bug 752434 or what should we do in this case?
3) Could you provide the revised tab-stroke-*.png images for OS X (+@2x) and Linux? They should be like Windows where they only contain the stroke and inner highlight.

Thanks

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 747309 [details] [diff] [review]
WIP v.2 - Windows and OS X

This approach looks OK to me.

(In reply to Matthew N. [:MattN] from comment #4)
> Created attachment 747309 [details] [diff] [review]
> WIP v.2 - Windows and OS X
> 
> I measured that the height of the tab above the navbar border (on Windows
> and OS X) is now 28px as specified by the spec. I haven't tested this on
> Linux yet.
> 
> Two ideas:
> A) I considered margin-top: -1px on the content-box but I wonder if that
> will cause perf problems (e.g. a reflow) or will introduce bugs.
> B) Use a 1px high fill that matches the bottom pixel color of the selected
> tab
> 

I'd opt for A, and not assume that it will cause reflow problems / perf problems unless we observe this to actually be the case.

Comment 7

[Dão Gottwald [:dao]]

I'm not quite sure what the two ideas refer to, i.e. what problem they're trying to solve. What does "content-box" mean? Is it .tab-content? The web content area? Something else? What element is B) referring to that would have the 1px high fill and what downsides could this have compared to A) where you already outlined concerns?

Comment 8

[Matthew N. [:MattN]]

Attachment: v.1 Add Linux changes and ignore disablechrome issue

(In reply to Dão Gottwald [:dao] from comment #7)
> I'm not quite sure what the two ideas refer to, i.e. what problem they're
> trying to solve. What does "content-box" mean? Is it .tab-content? The web
> content area? Something else? What element is B) referring to that would
> have the 1px high fill and what downsides could this have compared to A)
> where you already outlined concerns?

Sorry, the two ideas were for question 2 to Stephen. I reorganized the comment but forgot to move those ideas below. Now that 752434 and bug 870545 are in good shape, I don't think we need to worry about this case anymore.

I don't think we need to block this bug on the images from Stephen as I suspect we'll need to do a ui-review pass on all images.

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 749615 [details] [diff] [review]
v.1 Add Linux changes and ignore disablechrome issue

>+#TabsToolbar::after {
>+  content: '';
>+  position: absolute;
>+  bottom: 1px;
>+  left: 0;
>+  right: 0;
>+  z-index: 0;
>+  border-bottom: 1px solid hsla(0,0%,0%,.3);
> }

>-#TabsToolbar[tabsontop=true] {
>-  box-shadow: 0 -1px 0 rgba(0,0,0,.1) inset;
>-}

Why is the upper method preferable over the bottom one? It feels more heavyweight, and it seems that it might cause problems like it already did on OS X (bug 853415 comment 34).


Generally, I've lost track of what's up with this bug, specifically why the selected tab currently doesn't connect with the border. As far as I know, this used to work, then something broke it, and it's not clear to me that we need the complexity of this patch to fix it (instead of reverting the change that regressed this, for instance).

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 749615 [details] [diff] [review]
v.1 Add Linux changes and ignore disablechrome issue

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

For Linux, you're going to need to update the fbTabTexture, otherwise the stroke at the top of the selected tab looks a bit too light. Something like:

fbTabTexture linear-gradient(transparent 0px, transparent 2px, rgba(255, 255, 255, 0.65) 2px, rgba(255, 255, 255, 0.65) 3px, rgba(255, 255, 255, 0.3)), linear-gradient(transparent 0px, transparent 2px, -moz-dialog 2px, -moz-dialog)

seems to be sufficient for me locally.

Otherwise, this seems to do the job. Thanks for your work on this!

Comment 12

[Matthew N. [:MattN]]

Attachment: Screenshot highlighting .tab-background-start on a selected tab

(In reply to Dão Gottwald [:dao] from comment #9)
> Comment on attachment 749615 [details] [diff] [review]
> v.1 Add Linux changes and ignore disablechrome issue
> 
> >+#TabsToolbar::after {
> >+  content: '';
> >+  position: absolute;
> >+  bottom: 1px;
> >+  left: 0;
> >+  right: 0;
> >+  z-index: 0;
> >+  border-bottom: 1px solid hsla(0,0%,0%,.3);
> > }
> 
> >-#TabsToolbar[tabsontop=true] {
> >-  box-shadow: 0 -1px 0 rgba(0,0,0,.1) inset;
> >-}
> 
> Why is the upper method preferable over the bottom one? It feels more
> heavyweight, and it seems that it might cause problems like it already did
> on OS X (bug 853415 comment 34).

The nav-bar now covers up the old box-shadow and with the correct dimensions, the border needs to have a 1px space below it for the hightlight  (which is on the #nav-bar. Another reason is for consistency across platforms which makes it easier to make cross-platform changes. If you can think of a cleaner way to get the proper tab stroke seam, height and highlight, I'm open to suggestions.
This patch doesn't seem to cause the same problem as bug 853415 on Linux (probably because the tabs are not in the titlebar)

> Generally, I've lost track of what's up with this bug, specifically why the
> selected tab currently doesn't connect with the border. As far as I know,
> this used to work, then something broke it, and it's not clear to me that we
> need the complexity of this patch to fix it (instead of reverting the change
> that regressed this, for instance).

Problems with what is currently on UX:
A) The tab height doesn't match the image height (30px vs. 31px) which is leading to stretching
B) The tab stroke image has 1 px below the dark border at the bottom of the image because there is supposed to be an inner highlight stroke of 1px. It should connect with the highlight of the nav-bar and the top middle of the tab. This was never working because I made the original stroke images myself based on SVG paths Stephen sent me. This stroke image with the highlight still needs to be made by Stephen for OS X.

To have the selected tab's stroke and highlight connect with the border between the nav-bar and tab toolbar, the nav-bar is shifted upwards on top of the background tabs while the selected tab is topmost. I've attached a screenshot showing the bounds of the selected tab's tab-background-start with this patch.

Comment 13

[Matthew N. [:MattN]]

Attachment: v.1.1 Fix Linux fgTabTexture to match Windows stops

(In reply to Mike Conley (:mconley) from comment #11)
> Comment on attachment 749615 [details] [diff] [review]
> v.1 Add Linux changes and ignore disablechrome issue
> For Linux, you're going to need to update the fbTabTexture, otherwise the
> stroke at the top of the selected tab looks a bit too light.

I think this was mostly because the stops were wrong (there is 1px of mostly transparency above the top of the tabs). I changed the stops to match Windows and this looks better to me and still uses the colours from the spec IIRC.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 750265 [details] [diff] [review]
v.1.1 Fix Linux fgTabTexture to match Windows stops

LGTM! Great job on this!

-Mike

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 750265 [details] [diff] [review]
v.1.1 Fix Linux fgTabTexture to match Windows stops

>+#TabsToolbar::after {
>+  content: '';

nit: use double quotes

Comment 16

[Matthew N. [:MattN]]

Pushed with comment 15 addressed.

https://hg.mozilla.org/projects/ux/rev/cb56ba326fa7

Comment 17

[Stephen Horlander [:shorlander]]

Attachment: Tab Images Linux/OS X

> Stephen: 
> 1) What were the tab-texture-*.png images for in browser-aero.css? Was it
> just that the colour should change in that case? Any reason why we can't
> just override the colours in the linear-gradient instead of using an image?

I think it was the only way I could get it to layer properly. If you can do it another way it should be fine.


> 3) Could you provide the revised tab-stroke-*.png images for OS X (+@2x) and
> Linux? They should be like Windows where they only contain the stroke and
> inner highlight.

Should be able to use these on OS X and Linux.

Comment 18

[:Gijs (out for now; he/him)]

https://hg.mozilla.org/mozilla-central/rev/cb56ba326fa7