351372 - to make active tab look more "on top", tab edges should "flow" more into the white strip

Status: RESOLVED WORKSFORME

(Firefox :: Tabbed Browser, enhancement)

Description

[Matt]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1b2) Gecko/20060904 BonEcho/2.0b2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1b2) Gecko/20060904 BonEcho/2.0b2

edges of the active tab look too "sharp" when meeting with the white strip below the tabs.

Reproducible: Always

Steps to Reproduce:
1. open a couple of tabs
2.
3.




the active tab can look more "on top" if the tab corners are chamfered with the white strip below the tabs

Comment 1

[Matt]

Attachment: pic of active tab with "sharp" edges

Comment 2

[Matt]

Attachment: pic of active tab with slightly rounded edges (1 pxl taken out)

IMO the slightly rounded edges make the tab stand out just a tiny bit more than the others.

Comment 3

[Ria Klaassen (not reading all bugmail)]

Yeah, maybe an idea. 

Comment 4

[Dão Gottwald [:dao]]

This won't be easy, because:

- the tabs can grow vertically, which means you can't just add two white pixels in the tab images.

- with transparent tab images (attachment 236786 [details]), the whole effect is gone anyway. I don't have an idea how to fix that as yet.

Comment 5

[Ria Klaassen (not reading all bugmail)]

No, you need to redraw the images (the tab-left.png and tab-right.png) and put the tabs two pixels closer to each other to avoid a larger gab between the tabs I guess. I think it will looks better but the question is if it's worthwhile with all the 2.0 blocking bugs. :)

Comment 6

[Ria Klaassen (not reading all bugmail)]

Attachment: screenshot

This is what it would look like "in real". I've only done the right image of the active tab. I wonder if it would have implications with large font-settings, other dpi's etc.

Comment 7

[Peter Kasting]

(In reply to comment #6)
> This is what it would look like "in real". I've only done the right image of
> the active tab. I wonder if it would have implications with large
> font-settings, other dpi's etc.

Definitely do test those sorts of things; it's given us all sorts of pain so far.

If it can be made to work with those, I'd love to see this change, I think it's a nice bit of polish.

Setting target milestone; not requesting blocking yet because this is so minor.

Comment 8

[Peter Kasting]

Attachment: branch patch v1

This is a winstripe-only patch that should do what's necessary here.

The selected tab now gets a single-pixel-high row on the bottom of the left and right endcaps.  We use the existing endcap images, offset appropriately, to provide a "pushed out by one pixel" image for these rows.

The implementation here is flexible enough to support a couple of other images, which I also tried:
* The tabstrip-bottom image, repeated acrorss the row: This effectively meant we were cutting off 1 px of the selected tab's endcap gradient, and it didn't blend as well; the tab looked "bolted on" instead of flowing into the strip.
* The existing endcap images, but taking the bottom row instead of the 24th row: in theory this should handle large fonts better, but in practice the endcaps don't scale right anyway, and this just looked weird, much like the tabstrip-bottom image did but with a light pixel at the corners.

I purposefully did not patch Pinstripe since I'm not convinced this visual style change is desirable there.

Comment 9

[Dão Gottwald [:dao]]

(In reply to comment #8)
> We use the existing endcap images, offset appropriately, to
> provide a "pushed out by one pixel" image for these rows.

This leaves an uncovered pixel, doesn't it? If the endcap image has to cover 8px and is shifted by 1px, it would have to be 9px wide. A quick fix would be to decrease the visible width to 7px.

Comment 10

[Dão Gottwald [:dao]]

(In reply to comment #9)
> A quick fix would be to decrease the visible width to 7px.

Note: This would work, because the leftmost pixel column of the right endcap matches the middle image exactly. Otherwise there would be a potential mismatch when cutting this column off.

Comment 11

[Peter Kasting]

(In reply to comment #10)
> (In reply to comment #9)
> > A quick fix would be to decrease the visible width to 7px.
> 
> Note: This would work, because the leftmost pixel column of the right endcap
> matches the middle image exactly. Otherwise there would be a potential mismatch
> when cutting this column off.

It does leave an uncovered pixel.  This isn't an issue using the last row of the tab endcap images, since the neighboring pixels are "uncovered" (alpha 0%) anyway; and because of that, I didn't feel too bad about doing this even when using a source row from higher up in the image.

Changing the visible width would mean the endcaps would appear to move 1px closer to the favicon/close button, which is probably not a change we want to make.

The other option here is just to edit the endcap images, make them exactly 24 px high, and have the bottom of the active image do the "flow" itself.  This is a much safer fix and one that would stand a better chance of getting branch approval, but it makes it that much harder to (someday) fix the "endcaps don't scale with big fonts" issue.  Of course, who knows when that will be fixed.

Comment 12

[Dão Gottwald [:dao]]

(In reply to comment #11)
> It does leave an uncovered pixel.  This isn't an issue using the last row of
> the tab endcap images, since the neighboring pixels are "uncovered" (alpha 0%)
> anyway;

No, it's not transparent. The color at (0,23) is #f0f0f0 with an opacity of 15%.

> Changing the visible width would mean the endcaps would appear to move 1px
> closer to the favicon/close button, which is probably not a change we want to
> make.

I could make the images 9px wide then.

Comment 13

[Peter Kasting]

(In reply to comment #12)
> (In reply to comment #11)
> > It does leave an uncovered pixel.  This isn't an issue using the last row of
> > the tab endcap images, since the neighboring pixels are "uncovered" (alpha 0%)
> > anyway;
> 
> No, it's not transparent. The color at (0,23) is #f0f0f0 with an opacity of
> 15%.

I said the last row.  (0,74).

> > Changing the visible width would mean the endcaps would appear to move 1px
> > closer to the favicon/close button, which is probably not a change we want to
> > make.
> 
> I could make the images 9px wide then.

OK, that would work.  But for that matter, you could simply provide a separate pair of 1x8 images to be used in these particular locations.  That would perhaps enable the best blending of the edge lines, tab gradients, tab strip bottom, etc.  I don't know which approach is better.

Comment 14

[Dão Gottwald [:dao]]

Attachment: tab-left.png, 9px wide

Comment 15

[Dão Gottwald [:dao]]

Attachment: tab-right.png, 9px wide

Comment 16

[Dão Gottwald [:dao]]

(In reply to comment #13)
> I said the last row.  (0,74).

Right, but that's not visible anyway. ;)

> OK, that would work.  But for that matter, you could simply provide a separate
> pair of 1x8 images to be used in these particular locations.  That would
> perhaps enable the best blending of the edge lines, tab gradients, tab strip
> bottom, etc.  I don't know which approach is better.

Sounds even better to me. I'll attach new images.

Comment 17

[Dão Gottwald [:dao]]

(In reply to comment #16)
> (In reply to comment #13)
> > OK, that would work.  But for that matter, you could simply provide a separate
> > pair of 1x8 images to be used in these particular locations.  That would
> > perhaps enable the best blending of the edge lines, tab gradients, tab strip
> > bottom, etc.  I don't know which approach is better.
> 
> Sounds even better to me. I'll attach new images.

Err, no. This way, we'd be stuck again at a height of 24px.
I'm going to bed now, it's late in Germany. :)

Comment 18

[Peter Kasting]

(In reply to comment #16)
> (In reply to comment #13)
> > I said the last row.  (0,74).
> 
> Right, but that's not visible anyway. ;)

Perhaps you missed the part where I said I had tried that as one of my image source possibilities.  That's what I was referring back to.

(In reply to comment #17)
> (In reply to comment #16)
> > Sounds even better to me. I'll attach new images.
> 
> Err, no. This way, we'd be stuck again at a height of 24px.
> I'm going to bed now, it's late in Germany. :)

It wouldn't make us stuck any more than pulling the 24th row of the existing images (like my current patch does) would.  In fact, in theory, it can make things work better, if we can figure out a single 1 px high image that looks good no matter where in the gradiant we are above it.  That's what I'm trying to do now, but I'm stuck with picking pieces out of existing images.

Comment 19

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

Comment on attachment 238306 [details] [diff] [review]
branch patch v1

Definitely does look nicer, not a blocker, but very nice polish to take.

Comment 20

[Peter Kasting]

mconnor is leery of touching the tab strip any more than needed at this point, so this doesn't look like it will make Firefox 2.  Leaving approval request up for the moment in case he changes his mind or this is somehow deemed suitable for a 0.x release (ha, ha).

Comment 21

[Dão Gottwald [:dao]]

(In reply to comment #18)
> Perhaps you missed the part where I said I had tried that as one of my image
> source possibilities.  That's what I was referring back to.

Yes, I missed that. But I don't think that's an option anyway.

> It wouldn't make us stuck any more than pulling the 24th row of the existing
> images (like my current patch does) would.  In fact, in theory, it can make
> things work better, if we can figure out a single 1 px high image that looks
> good no matter where in the gradiant we are above it.  That's what I'm trying
> to do now, but I'm stuck with picking pieces out of existing images.

What about a bright one-pixel image that overlays the corner (instead of replacing the bottom row)? I'll make a mockup of what it would look like.

Comment 22

[Dão Gottwald [:dao]]

Attachment: tab-bottom-corner.png

Comment 23

[Dão Gottwald [:dao]]

Attachment: mockups using tab-bottom-corner.png

Comment 24

[Peter Kasting]

(In reply to comment #23)
> Created an attachment (id=238385) [edit]
> mockups using tab-bottom-corner.png

That doesn't look right at all to me.  On dark themes, for example, you don't get the right effect.

Comment 25

[Dão Gottwald [:dao]]

(In reply to comment #24)
> On dark themes, for example, you don't get the right effect.

Yes. Nevertheless I think it's an improvement -- the "before" screenshot looks worse, because of the gap between the tab's and the strip's borders.

Could you attach screenshots of your current solution? Maybe my concern because of the transparent pixel isn't visible at all. And I'm interested in a high-contrast screenshot, too.

Comment 26

[Peter Kasting]

(In reply to comment #25)
> Could you attach screenshots of your current solution? Maybe my concern because
> of the transparent pixel isn't visible at all. And I'm interested in a
> high-contrast screenshot, too.

Sadly, I only have the ability to screenshot this on GNOME, where I haven't figured out how to make my colors high-contrast.  Since my patch requires no new images, do you think you could drop it into a local build and screenshot it on Windows?  That would be really awesome :D

Comment 27

[Dão Gottwald [:dao]]

Attachment: patch v1 screenshots

The transparent pixel is visible in the high-contrast screenshot. But even if you ignore that, I think my mockups look better ;)

Comment 28

[Peter Kasting]

(In reply to comment #27)
> Created an attachment (id=238512) [edit]
> patch v1 screenshots
> 
> The transparent pixel is visible in the high-contrast screenshot. But even if
> you ignore that, I think my mockups look better ;)

I don't, but I don't think my patch looks as good as it could either.  I still want to see a pair of 1x8 images to insert here that get the blending "correct"; on a dark theme, the "corner" pixel shouldn't be as dark as in my mockup or as light as in yours, and the very edge pixel should blend better with the tab strip border color.

Comment 29

[Dão Gottwald [:dao]]

Attachment: tab-left-bottom.png

Comment 30

[Dão Gottwald [:dao]]

Attachment: tab-right-bottom.png

Comment 31

[Dão Gottwald [:dao]]

Attachment: screenshots using tab-left-bottom.png & tab-right-bottom.png

Comment 32

[Peter Kasting]

(In reply to comment #31)
> Created an attachment (id=238523) [edit]
> screenshots using tab-left-bottom.png & tab-right-bottom.png

Yeah, that's closer to what I was thinking, though I was imagining the endmost pixel of those 1x8 images would look "halfway between" the tabstrip-bottom border and the white border in the tab endcaps; as-is, it looks like it matches the tabstrip-bottom exactly.

Either way, I think it's better than either of our previous patches.

Comment 33

[Dão Gottwald [:dao]]

Attachment: tab-left-bottom.png & tab-right-bottom.png v2

I think this looks worse. With the Royale theme, the outmost pixel seems too dominant. With high contrast, the benefit is questionable.

Comment 34

[Peter Kasting]

(In reply to comment #33)
> Created an attachment (id=238529) [edit]
> tab-left-bottom.png & tab-right-bottom.png v2
> 
> I think this looks worse. With the Royale theme, the outmost pixel seems too
> dominant. With high contrast, the benefit is questionable.

That doesn't do remotely like what I wanted.  I was thinking this:

 |W...
 |W...
-xz...

Here, the | is the dark edge of the tab, the W is the white column adjacent, ... is the body of the tab, - is the top border of the bottom strip, and "xz" are the left two pixels of tab-left-bottom.png.

My desire was that in ALL themes, x would be halfway between the value of - and W. z would be toned down to some dimmer value, much as you're already doing.

Instead, in your screenshot, x is very close to W on Royale, and very close to - in high contrast mode.

Comment 35

[Dão Gottwald [:dao]]

(In reply to comment #34)
> Instead, in your screenshot, x is very close to W on Royale, and very close to
> - in high contrast mode.

That's because "-" is a native color (ThreeDShadow), whose difference between Royale and high contrast is minor -- whereas the difference of the background colors is huge.

Maybe I could come up with something that works better in this case (e.g. use a less-opaque gray for "x" that inherits less from the background), but then there's a potential mismatch with other shades of ThreeDShadow. To be honest, this scares me. My first attempt of 1x8 images is probably much safer.

Comment 36

[Mike Connor [:mconnor]]

Comment on attachment 238306 [details] [diff] [review]
branch patch v1

we're done with the tabstrip for now

Comment 37

[Mike Connor [:mconnor]]

Comment on attachment 238306 [details] [diff] [review]
branch patch v1

this is almost certainly bitrotted, please request a review on a new patch, if possible, from any of the reviewers listed at http://www.mozilla.org/projects/firefox/review.html

Comment 38

[:Felipe Gomes (needinfo for replies!)]

I'm pretty sure this has been fixed by various tweaks through the years, and the Australis work (bug 738491) incorporates this idea even more strongly.