547787 - New style for the tab bar

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Markus Stange [:mstange]]

Attachment: wip

Comment 1

[Dão Gottwald [:dao]]

Could border-image be used here and would it result in more maintainable code? A glance at this patch makes me wonder whether the Firefox 3 implementation wasn't actually easier to deal with...

Comment 2

[Markus Stange [:mstange]]

Using border-image would mean that we need a separate image per state. Using -moz-image-rect we can put the whole tab bar design in one image, and we might even reuse the CSS code for winstripe just with a different tabbar.png.
I agree that it's messy. I can put the code away into a separate file so that we don't have to look at it very often... :)

Comment 3

[Dão Gottwald [:dao]]

The problem is not looking at it this code but the idea of modifying it.

Using one image per state doesn't sound like a showstopper to me, since winstripe is doing it as well.

Comment 4

[Markus Stange [:mstange]]

I need to think about that a little.

Tryserver build is here:
https://build.mozilla.org/tryserver-builds/[e-mail]-try-960e374df989/try-960e374df989-macosx.dmg

Comment 5

[Dão Gottwald [:dao]]

The lightweight theme styling looks slightly unintentional. Scrolling doesn't work as expected (scrolls too far) because of the negative margins. The negative margins also cause the wrong tab to be selected when clicking on a tab's right edge. Last but not least, I'm getting infinite overflow/underflow loops with 5-10 tabs open in a 1024 px wide window (due to the margins, I guess). I think the negative margins could actually be dropped without loosing much. The tabs don't seem to overlap visually, except for the active tab.

Comment 6

[Markus Stange [:mstange]]

(In reply to comment #5)
> The lightweight theme styling looks slightly unintentional.

Yeah, I didn't pay attention to the lwtheme styling at all. We probably need to fall back to the current border-colors styling for the lwtheme case until we have bug 547805, unless somebody has a better idea.

> Scrolling doesn't
> work as expected (scrolls too far) because of the negative margins.
> Last but not least, I'm getting infinite overflow/underflow
> loops with 5-10 tabs open in a 1024 px wide window (due to the margins, I
> guess).

I only had these problems before I set the outer margins on the first and last tab to zero. I can't reproduce them now. How did you test the patch?

> The
> negative margins also cause the wrong tab to be selected when clicking on a
> tab's right edge.

I think we could fix that by changing the XUL to wrap the tab's contents into a hbox. Then we could set the margins on that hbox and not have them affect the clickable rect (since the tab binding uses display="xul:button").

> I think the negative margins could actually be dropped without loosing
> much. The tabs don't seem to overlap visually, except for the active tab.

Right, the active tab is the problem. I haven't worked out a simpler way to make that work yet, but I'll think about it some more. All the approaches I've thought of will either compromise the rounded edge of the active tab or they force us to do drop the hover effect, or to do nasty alpha trickery with the images...

Comment 7

[Dão Gottwald [:dao]]

(In reply to comment #6)
> > Scrolling doesn't
> > work as expected (scrolls too far) because of the negative margins.
> > Last but not least, I'm getting infinite overflow/underflow
> > loops with 5-10 tabs open in a 1024 px wide window (due to the margins, I
> > guess).
> 
> I only had these problems before I set the outer margins on the first and last
> tab to zero. I can't reproduce them now. How did you test the patch?

I was using the tryserver build.

Comment 8

[Markus Stange [:mstange]]

I think I've thought of a slightly simpler approach involving ::before and ::after. That way we can avoid multiple background images and make more use of the cascade.

Comment 9

[Markus Stange [:mstange]]

Attachment: wip

This looks better, but I had to make changes to scrollbox.xml. I still need to fix lwtheme styling, extract the correct tab images from the PSDs and find out why I'm hitting the assertion "Invalid computed width: 'aComputedWidth >= 0', file /Users/markus/code/mozilla/layout/generic/nsHTMLReflowState.cpp, line 229".

Comment 10

[Markus Stange [:mstange]]

Attachment: wip

This fixes the lightweight theme styling. I've solved the active tab problem by inserting a flexing spacer into the tabstrip, and moving the tabstrip background to this spacer and to the other buttons in the tabstrip. This requires much of the button style to move to the toolbarbutton-icon, which is a little ugly...

This patch also suffers from a bug where the scroll position isn't adjusted properly when a tab closes without causing underflow. I still need to look into that and into the other things that I mentioned last time.

Comment 11

[Markus Stange [:mstange]]

Attachment: wip

I think this is almost ready. I only need to reorder some rules, add more comments and extract the non-CSS parts into separate bugs.

Tryserver build:
https://build.mozilla.org/tryserver-builds/[e-mail]-try-aec49c7a0c36/try-aec49c7a0c36-macosx.dmg

Comment 12

[Markus Stange [:mstange]]

Attachment: v1

Comment 13

[Markus Stange [:mstange]]

I'm going to fix tabs-on-top in a different bug. The patch in this bug just flips the tab bar upside down in tabs-on-top mode, but that's not what they're supposed to look like.

Comment 14

[Stephen Horlander [:shorlander]]

Attachment: New Tab Clipped

(In reply to comment #12)
> Created an attachment (id=438734) [details]
> v1

Visually it looks great! Only thing I noticed was that the new tab "tab" looks clipped.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 438734 [details] [diff] [review]
v1

>+        <xul:spacer class="tabs-end-spacer"/>

This seems to break opening new tabs with a double click.

- I think the label was moving when selecting a tab, not sure why.

- I don't really see a reason to use background images rather than border image, I think the latter is the way to go.

- The overflow new tab button seems to lack a hover state.

Comment 16

[Markus Stange [:mstange]]

(In reply to comment #15)
> (From update of attachment 438734 [details] [diff] [review])
> >+        <xul:spacer class="tabs-end-spacer"/>
> 
> This seems to break opening new tabs with a double click.

Indeed. Any idea how to fix?

> - I think the label was moving when selecting a tab, not sure why.

Because the close button became visible in that tab? I think that was an issue before this patch, too.

> - I don't really see a reason to use background images rather than border
> image, I think the latter is the way to go.

Alright.

> - The overflow new tab button seems to lack a hover state.

Good catch.

Comment 17

[Dão Gottwald [:dao]]

(In reply to comment #16)
> > >+        <xul:spacer class="tabs-end-spacer"/>
> > 
> > This seems to break opening new tabs with a double click.
> 
> Indeed. Any idea how to fix?

There's a dblclick handler in tabbrowser.xml that checks event.originalTarget.localName == "box". Looks like that could be extended easily.

Completely unrelated: The way you compose the toolbar background might become a problem in combination with bug 457187, i.e. with other elements on the toolbar.

Comment 18

[Markus Stange [:mstange]]

(In reply to comment #15)
> (From update of attachment 438734 [details] [diff] [review])
> - I don't really see a reason to use background images rather than border
> image, I think the latter is the way to go.

I've found a reason: more control over the mouse hit region. I need the left half of the connection image between two tabs to extend outside the tab so that clicking it won't click the right tab. I don't see a way to do that with border-image at the moment. Do you?

Also, I'm still not convinced that using border-images would save a significant amount of code. We'd get rid of this part:

> .tabbrowser-tab,
> .tabbrowser-arrowscrollbox > .tabs-newtab-button {
>   background-image: -moz-image-rect(url(chrome://browser/skin/tabbrowser/tabbar.png), 0, 31, 26, 28);
> }
>
> .tabbrowser-tab:hover,
> .tabbrowser-arrowscrollbox > .tabs-newtab-button:hover {
>   background-image: -moz-image-rect(url(chrome://browser/skin/tabbrowser/tabbar.png), 0, 101, 26, 98);
> }
>
> .tabbrowser-tab[selected="true"] {
>   background-image: -moz-image-rect(url(chrome://browser/skin/tabbrowser/tabbar.png), 0, 17, 26, 14);
> }

because the middle part would be embedded in the border image; but we'd still have to set a different image for every possible combination of adjacent tab states. And adapting that for RTL wouldn't be as straightforward either.

(One image per tab state isn't enough since the tabs overlap each other, especially the selected tab.)

(In reply to comment #17)
> (In reply to comment #16)
> > > >+        <xul:spacer class="tabs-end-spacer"/>
> > > 
> > > This seems to break opening new tabs with a double click.
> > 
> > Indeed. Any idea how to fix?
> 
> There's a dblclick handler in tabbrowser.xml that checks
> event.originalTarget.localName == "box". Looks like that could be extended
> easily.

I'll look into that.

> Completely unrelated: The way you compose the toolbar background might become a
> problem in combination with bug 457187, i.e. with other elements on the
> toolbar.

Let's look at that after bug 457187.

Comment 19

[Dão Gottwald [:dao]]

(In reply to comment #18)
> I've found a reason: more control over the mouse hit region. I need the left
> half of the connection image between two tabs to extend outside the tab so that
> clicking it won't click the right tab. I don't see a way to do that with
> border-image at the moment. Do you?

Any region that should select the right tab should be part of the right tab, and the same goes for the left tab. I don't think there should be an area that's attributed to neither tab.

> Also, I'm still not convinced that using border-images would save a significant
> amount of code. We'd get rid of this part:

I don't care about the amount of code as much as I dislike the struggling with the image rects and the generated content...

> > Completely unrelated: The way you compose the toolbar background might become a
> > problem in combination with bug 457187, i.e. with other elements on the
> > toolbar.
> 
> Let's look at that after bug 457187.

I'd rather understand the implications beforehand.

Comment 20

[Markus Stange [:mstange]]

(In reply to comment #19)
> (In reply to comment #18)
> > I've found a reason: more control over the mouse hit region. I need the left
> > half of the connection image between two tabs to extend outside the tab so that
> > clicking it won't click the right tab. I don't see a way to do that with
> > border-image at the moment. Do you?
> 
> Any region that should select the right tab should be part of the right tab,
> and the same goes for the left tab.

Let's take this example: LTR mode, |normal tab | | selected tab |
The selected tab has an outward curve in its top left corner that overflows its clickable box into the clickable box of the normal tab.
The left tab doesn't know that there's a selected tab next to it, so it can't provide the curved corner. So that corner necessarily has to be part of the right tab somehow -- but outside of its clickable rect.

The current patch does it like this (assuming LTR):
 - Every tab provides the complete 11px-wide tab separator image between
   itself and the tab left of it.
 - The rightmost tab also provides its end separator image because there's
   no tab after it that would take care of it.
 - The left half of the separator image extends beyond the tab and overlays
   the rightmost part of the left tab.
 - The separator image has pointer-events: none, so clicking the left half of
   it (which is over the left tab) will still activate the left tab.
 - Every tab reserves transparent space at the right so that the separator
   image of the next tab has a transparent base (so that it works with
   Personas).

> I don't think there should be an area
> that's attributed to neither tab.

That's not what I'm talking about. There's no space between the tabs.

> > Also, I'm still not convinced that using border-images would save a significant
> > amount of code. We'd get rid of this part:
> 
> I don't care about the amount of code as much as I dislike the struggling with
> the image rects and the generated content...

Aha!
I can understand the image rect concern. But what's bad about the generated content? That you can't inspect it with the DOM Inspector?

Image rects also have the advantage of preloading all required states. With border-image we would probably see a short flash when hovering over a tab for the first time, assuming we don't do any preload hacks.

> > > Completely unrelated: The way you compose the toolbar background might become a
> > > problem in combination with bug 457187, i.e. with other elements on the
> > > toolbar.
> > 
> > Let's look at that after bug 457187.
> 
> I'd rather understand the implications beforehand.

OK, I'll play with that patch.

Comment 21

[Dão Gottwald [:dao]]

(In reply to comment #20)
> The left tab doesn't know that there's a selected tab next to it

It does, the beforeselected tells it.

> Aha!
> I can understand the image rect concern. But what's bad about the generated
> content? That you can't inspect it with the DOM Inspector?

Sure, DOM Inspector is part of the story. I also dislike the image tiling and margin hacking. All that might be necessary sometimes, but it doesn't seem to be in this case.

> Image rects also have the advantage of preloading all required states. With
> border-image we would probably see a short flash when hovering over a tab for
> the first time, assuming we don't do any preload hacks.

With multiple background images I guess we could preload fairly easily...

Comment 22

[Markus Stange [:mstange]]

(In reply to comment #21)
> (In reply to comment #20)
> > The left tab doesn't know that there's a selected tab next to it
> 
> It does, the beforeselected tells it.

OK, time for another thinking session.

Comment 23

[Markus Stange [:mstange]]

Just to make sure I understood what you want: We'll need the following border-images, right?

 1  empty      | normal   | unselected
 2  unselected | normal   | empty
 3  empty      | normal   | selected
 4  selected   | normal   | empty
 5  unselected | normal   | unselected
 6  unselected | normal   | selected
 7  selected   | normal   | unselected
 8  empty      | hover    | unselected
 9  unselected | hover    | empty
10  empty      | hover    | selected
11  selected   | hover    | empty
12  unselected | hover    | unselected
13  unselected | hover    | selected
14  selected   | hover    | unselected
15  empty      | selected | unselected
16  unselected | selected | empty
17  unselected | selected | unselected

These are the left, middle and right parts of a 6px - stretch - 5px border-image. "unselected" means "either normal or hover, but we don't care because those states don't extend beyond their rect".
The states with "empty" in their left / right part are for the first tab in LTR / RTL mode.

This doesn't look much simpler, so I probably still don't understand what you have in mind.

Comment 25

[Markus Stange [:mstange]]

Dão, I'm waiting for an answer to comment 23 before continuing.

Comment 26

[Dão Gottwald [:dao]]

If the difference between "empty" and "unselected" is about 2 semi-transparent black pixels, then those probably don't need to be part of the tab, but could be drawn in the background. A dedicated hover state seems unnecessary where the tab isn't adjacent to the selected tab. These are just random thoughts, there might be more room for optimizations.

Comment 27

[Markus Stange [:mstange]]

(In reply to comment #26)
> If the difference between "empty" and "unselected" is about 2 semi-transparent
> black pixels, then those probably don't need to be part of the tab, but could
> be drawn in the background.

But those 2 semi-transparent pixels differ dependent on whether the first tab is selected or not.

> A dedicated hover state seems unnecessary where the
> tab isn't adjacent to the selected tab.

I don't understand this. Which of the states listed above would this eliminate?

Comment 28

[Dão Gottwald [:dao]]

(In reply to comment #27)
> (In reply to comment #26)
> > If the difference between "empty" and "unselected" is about 2 semi-transparent
> > black pixels, then those probably don't need to be part of the tab, but could
> > be drawn in the background.
> 
> But those 2 semi-transparent pixels differ dependent on whether the first tab
> is selected or not.

How much do they differ?

> > A dedicated hover state seems unnecessary where the
> > tab isn't adjacent to the selected tab.
> 
> I don't understand this. Which of the states listed above would this eliminate?

Those involving "hover" but not "selected".

Comment 29

[Markus Stange [:mstange]]

Attachment: clipped selected first tab

(In reply to comment #28)
> (In reply to comment #27)
> > (In reply to comment #26)
> > > If the difference between "empty" and "unselected" is about 2 semi-transparent
> > > black pixels, then those probably don't need to be part of the tab, but could
> > > be drawn in the background.
> > 
> > But those 2 semi-transparent pixels differ dependent on whether the first tab
> > is selected or not.
> 
> How much do they differ?

It would look like in this picture. It's clearly noticeable.
If I instead make the left part of the border image 6px and the right part 5px, then it's less noticeable, but then the curve between the selected tab and the new tab button suffers in the same way, because the new tab button can't adjust its state dependent on whether the last tab is selected.

> > > A dedicated hover state seems unnecessary where the
> > > tab isn't adjacent to the selected tab.
> > 
> > I don't understand this. Which of the states listed above would this eliminate?
> 
> Those involving "hover" but not "selected".

What state would hovered tabs that are not next to the selected tab use instead? I still don't understand, sorry.

Comment 30

[Dão Gottwald [:dao]]

(In reply to comment #29)
> > > But those 2 semi-transparent pixels differ dependent on whether the first tab
> > > is selected or not.
> > 
> > How much do they differ?
> 
> It would look like in this picture. It's clearly noticeable.
> If I instead make the left part of the border image 6px and the right part 5px,
> then it's less noticeable, but then the curve between the selected tab and the
> new tab button suffers in the same way, because the new tab button can't adjust
> its state dependent on whether the last tab is selected.

Another option would be using :before only for that small knob. That seems more acceptable than using it as excessively as the current patch does.

> What state would hovered tabs that are not next to the selected tab use
> instead? I still don't understand, sorry.

I was thinking of opacity. Winstripe uses a -moz-linear-gradient background image.

Comment 31

[Markus Stange [:mstange]]

(In reply to comment #30)
> Another option would be using :before only for that small knob. That seems more
> acceptable than using it as excessively as the current patch does.

I'll try that.
I still don't like having to slice the images manually instead of letting -moz-image-rect do the hard work, but I'll give it a go.

> > What state would hovered tabs that are not next to the selected tab use
> > instead? I still don't understand, sorry.
> 
> I was thinking of opacity.

I can't use that because that would change the opacity of the background, too.

Comment 32

[Markus Stange [:mstange]]

(In reply to comment #30)
> Another option would be using :before only for that small knob.

I tried this but the problem is that the first tab looks different even in some pixels that are contained in the border image. So I had to include the right part of the left edge of the first tab in the ::before image, too, and hide the left part of the border image by setting border-left-width to 0 and make up the space by a margin-left. And this means that the first tab is now wider than the other tabs, and that we need a separate first-tab knob for the hover state, too.

Comment 33

[Markus Stange [:mstange]]

Attachment: v2, still ::before images

This is a patch that still uses ::before and -moz-image-rect but fixes all the problems you and Stephen noticed with v1.

Comment 34

[Markus Stange [:mstange]]

Attachment: partial transition to border images

This goes the border-image route, at least partially. It's about 40 lines of code less, still has to resort to ::before, and already adds 13 images. And I haven't converted the scrollbuttons and the new-tab and alltabs buttons yet; then we'll end up with even more images.
And when we add a new appearance for the tabs-on-top state, the number of images will double. Same for a different background-window state, should we want one.

I really prefer slicing the images using CSS. I understand your concerns but I think it's still preferable to loads of image files.

Comment 35

[Markus Stange [:mstange]]

Attachment: partial transition to border images

There was a small mistake in the last patch.

Comment 36

[Markus Stange [:mstange]]

Attachment: v3

I've pulled the image setting rules out to a separate file (tabbar-images.inc) so that we can use preprocessor magic for tabs-on-top.

Comment 37

[Dão Gottwald [:dao]]

(In reply to comment #31)
> > > What state would hovered tabs that are not next to the selected tab use
> > > instead? I still don't understand, sorry.
> > 
> > I was thinking of opacity.
> 
> I can't use that because that would change the opacity of the background, too.

Make the background a CSS gradient with certain alpha values and increase them as needed on hover? :)

Comment 38

[Markus Stange [:mstange]]

Heh. Yeah, I suppose there is a way to make that work. But it would need a lot of experimenting, and I don't see how this will be easier to maintain than simply dropping in a new image when the tab bar style changes.
Also, it would probably decrease the saturation in the Personas case more than necessary because both black and white will get composited into the original color value, but that probably doesn't matter much.

Right now I don't have to deal with any alpha computations. I just make a greyscale image of the tab bar by cutting up the mockup, and then I plug that into a small tool that will spit out the finished alpha-transparent black and white image, which then gets dropped in the tabbrowser theme folder.

Comment 39

[Dão Gottwald [:dao]]

(In reply to comment #38)
> Heh. Yeah, I suppose there is a way to make that work. But it would need a lot
> of experimenting, and I don't see how this will be easier to maintain than
> simply dropping in a new image when the tab bar style changes.

Maybe I'm missing something; you already have the initial RGBA values in your image, and calculating the extra alpha value should be trivial, as the tab opacity is only changing factor. It's a lot easier to maintain, as merging the background and tab pixels multiplies the needed images or image slices as well as the corresponding code.

Comment 40

[Markus Stange [:mstange]]

Here's the tool that I've been using for making the images:
http://tests.themasta.com/greytracer/

(In reply to comment #39)
> (In reply to comment #38)
> > Heh. Yeah, I suppose there is a way to make that work. But it would need a lot
> > of experimenting, and I don't see how this will be easier to maintain than
> > simply dropping in a new image when the tab bar style changes.
> 
> Maybe I'm missing something; you already have the initial RGBA values in your
> image, and calculating the extra alpha value should be trivial, as the tab
> opacity is only changing factor.

Man, this is creating a knot in my head.
You're right again. But there's another thing in our way, which I hadn't thought about before:
Opacity disables sub-pixel anti-aliasing for the tab text, which doesn't look good (see also bug 562865).
Also, having the gradients in the CSS repeats info from the image which needs to be kept in sync. And I'd have to feed different backgrounds into the greytracer tool for selected vs. unselected tabs, which again complicates the process.

> It's a lot easier to maintain, as merging the
> background and tab pixels multiplies the needed images or image slices as well
> as the corresponding code.

I don't agree.

Comment 41

[Dão Gottwald [:dao]]

Keeping the gradient in sync (i.e. copying and pasting a few RGBA values) isn't any harder than updating all the images with the background being merged in (i.e. manually re-merging the tab textures with the background).

> And I'd have to feed different backgrounds into the
> greytracer tool for selected vs. unselected tabs, which again complicates the
> process.

In other words, you need the tab textures from Stephen without the background being merged in (the way you'd need them when changing the background)?

Do we have a platform bug on the text rendering quality?

Comment 42

[Markus Stange [:mstange]]

(In reply to comment #41)
> Keeping the gradient in sync (i.e. copying and pasting a few RGBA values) isn't
> any harder than updating all the images with the background being merged in
> (i.e. manually re-merging the tab textures with the background).

I'm not sure what to answer. Of course nothing is really "hard"; it seems the problem is just that we have different preferences about these things. So let's rather focus on the technical issues.

> > And I'd have to feed different backgrounds into the
> > greytracer tool for selected vs. unselected tabs, which again complicates the
> > process.
> 
> In other words, you need the tab textures from Stephen without the background
> being merged in (the way you'd need them when changing the background)?

I don't need them, I already have them; it's all on different layers in the Photoshop files.

I think you're saying that I shouldn't need to post-process any images to make them alpha-transparent but instead just use the transparent tabs from the PSD file, cut them up and drop them in as they are. Is that correct? Because then I don't know how this would address the part of my comment you quoted, because that way just won't work for the active tab.

> Do we have a platform bug on the text rendering quality?

Maybe, but not that I know of, and I don't think there's anything we can do about it. Subpixel-antialiasing just doesn't fit into the RGBA compositing model.

---

This discussion is taking up quite an amount of time I'd like to spend elsewhere. We have a patch here that is visually correct and well-understood and contains unfortunate but manageable and self-contained complexity. Can we now move on, please? If you really absolutely disagree with the approach, feel free to take over the bug and see if you succeed with a simpler way without making visual compromises.

Comment 43

[Markus Stange [:mstange]]

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

I thought about how to implement tab moving animation with this patch and came to the conclusion that it's unnecessarily hard to do so: Both the fact that a tab's right edge isn't owned by the tab itself (but by the next tab) and the fact that the tab strip background is only painted by things that sit atop it lead to complications.

I'll address this in the following ways:
 - Don't care about the active-tab-on-persona problem now. Let's wait for
   bug 547805.
 - Set the tabstrip background on #TabsToolbar and don't add the spacer.
 - Set background-color: -moz-mac-chrome-(in)active on all tab parts.
 - Don't share tab edges. Instead make every tab own its own edges by giving
   every tab a 5px ::before and 6px ::after tile. This is basically recreating
   border-image functionality but allows addressing the image parts separately
   and using -moz-image-rect.

Comment 44

[Dão Gottwald [:dao]]

(In reply to comment #43)
>  - Don't care about the active-tab-on-persona problem now. Let's wait for
>    bug 547805.

I'm okay with not having it highly polished, but it needs to look somewhat reasonable, since personas are a built-in feature and -moz-element is vaporware right now.

Comment 45

[Markus Stange [:mstange]]

Attachment: v4

with those changes, and I also added some documentation

Comment 46

[Markus Stange [:mstange]]

Attachment: screenshot of tabbar with personas

(In reply to comment #44)
> (In reply to comment #43)
> >  - Don't care about the active-tab-on-persona problem now. Let's wait for
> >    bug 547805.
> 
> I'm okay with not having it highly polished, but it needs to look somewhat
> reasonable

That's what it looks like now. It's not really obvious which tab is selected, unfortunately... Any ideas?

Comment 47

[d]

(In reply to comment #46)
> Created an attachment (id=447533) [details]
> screenshot of tabbar with personas
> 
> (In reply to comment #44)
> > (In reply to comment #43)
> > >  - Don't care about the active-tab-on-persona problem now. Let's wait for
> > >    bug 547805.
> > 
> > I'm okay with not having it highly polished, but it needs to look somewhat
> > reasonable
> 
> That's what it looks like now. It's not really obvious which tab is selected,
> unfortunately... Any ideas?

It might be an illusion, but I think they look a bit darker than the selected tab already. Perhaps we could darken them down just a bit more, to get a good difference. Would an rgba included as another background (multiple backgrounds) be able to do that?

Comment 48

[Dão Gottwald [:dao]]

We could keep the legacy styling for personas. Some earlier patch did that, I think.

The screenshot indicates that the toolbar background is duplicated on the background tabs and new tab button. Why's that the case?

Comment 49

[Markus Stange [:mstange]]

(In reply to comment #48)
> We could keep the legacy styling for personas. Some earlier patch did that, I
> think.

Yeah. I'll see what I can do about bug 506826 first, though.

> The screenshot indicates that the toolbar background is duplicated on the
> background tabs and new tab button. Why's that the case?

Because they include the toolbar background, so it's effectively visible twice: once from #TabsToolbar, once from the tab images themselves. They include the toolbar background because they're designed to sit on top of the original background that is under the toolbar background. Anything else wouldn't work for the selected tab.

Comment 50

[Dão Gottwald [:dao]]

(In reply to comment #49)
> > The screenshot indicates that the toolbar background is duplicated on the
> > background tabs and new tab button. Why's that the case?
> 
> Because they include the toolbar background, so it's effectively visible twice:
> once from #TabsToolbar, once from the tab images themselves. They include the
> toolbar background because they're designed to sit on top of the original
> background that is under the toolbar background. Anything else wouldn't work
> for the selected tab.

Right, so what you need to do is set the background color on the selected tab but make all other elements transparent.

Comment 51

[Markus Stange [:mstange]]

No, since "all other elements" also include edges of tabs that are adjacent to the selected tab. I also don't see what would benefit from that.

Comment 52

[Markus Stange [:mstange]]

Attachment: v5

Changes:
 - slightly better look in lwtheme mode by not setting the #TabsToolbar
   background in that case.
 - scrollbuttons and toolbarbuttons in the tab bar use borders and background
   colors again, no image regions
 - selectors and image rect positions in tabbar-images.inc are explained by
   generator script

I think this is more understandable, though not necessarily less complex. Does this address your concerns with this approach?

Comment 53

[Markus Stange [:mstange]]

Attachment: screenshot of tabbar with personas

Appearance with the v5 patch.

Note that we won't have this problem in tabs-on-top mode because there the selected tab doesn't erase darkness. So in the future default Personas will look good.

Comment 54

[Markus Stange [:mstange]]

Comment on attachment 452215 [details] [diff] [review]
v5

This has some problems with the tab opening animation. I'll fix them and then ask for review, but I'd still like your feedback on the generator approach.

Comment 55

[Markus Stange [:mstange]]

Attachment: v6

The problem during the new tab animation was that the things inside the tab (favicon, label and close button) were pushing the ::before and ::after boxes outside of the tab. Now I just prevent them from taking up space while they're invisible.

Comment 56

[Markus Stange [:mstange]]

Attachment: v7

The generator was broken in the previous patch.

Comment 57

[d]

While it's not a blocker, could this make it into Beta 1 for testing?

Comment 58

[Markus Stange [:mstange]]

Dão has a lot of other things on his plate, and it's a complicated patch, so it will probably not make Beta 1.

Comment 59

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

Marking as a beta2 blocker in order to expedite review: OSX users kept trying tabs-on-top in beta1, and the desire to have it was one of the most prominent bits of feedback.

Comment 60

[Dão Gottwald [:dao]]

Comment on attachment 453162 [details] [diff] [review]
v7

This doesn't seem to work for app tabs ... even worse, I don't see why.

Comment 61

[Markus Stange [:mstange]]

Looks like position:fixed is at fault again. I'll see what I can do.

Comment 62

[Markus Stange [:mstange]]

The patch in bug 579776 fixes the app tab problem.

Comment 63

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

Moving this to beta3, :(

Comment 64

[Markus Stange [:mstange]]

Attachment: v8

The problem with app tabs was simple: App tabs are display:block, and the ::before and ::after images also had display:block. So everything was vertically stacked on top of each other.
I now use display:inline-block on the ::before and ::after parts.

This patch now also includes tabs-on-top mode.

Comment 65

[Markus Stange [:mstange]]

Tryserver build: http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-72e2ce422690/tryserver-macosx/firefox-4.0b3pre.en-US.mac.dmg

Comment 67

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

Dao, will this make it for b3 code freeze on Monday, Aug 2?

Comment 68

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

I'm quite upset that this has gone several days without a comment; I assume that means it needs to move to beta4.

I expect this to land *EARLY* in the beta4 cycle.

Comment 69

[Johnathan Nightingale [:johnath]]

Dao, Markus - this was originally a blocker for tabs on top on mac, but that was solved elsewhere. What is the net effect of the work remaining here? I understood it to be chiefly about doing things the "right" way, whereas tabs on top in beta 2 was a bit of a hack-of-the-moment. On the other hand, you guys seem pretty clearly to be disagreeing about whether the implementation here is better.

If we spend the time to get through the implementation questions here and land the thing, can one of you clarify the win, for me? I believe this may not be the blocker it once was, but I am worried that I'm missing context.

Comment 70

[Markus Stange [:mstange]]

It makes the tabs match the mockups on https://wiki.mozilla.org/Firefox/4.0_Mac_Theme_Mockups - both tabs-on-top and tabs-on-bottom mode - which a considerable upgrade in visual quality.

The most obvious differences to the current state are the rounded corners connecting the tabs to the toolbars and the more refined borders and shadows.

I think these differences are very important. Someone from UX can probably justify them better than I can; maybe Stephen, who designed them in the first place.

The tryserver build from comment 65 still exists; feel free to play around with it to get a feel for the difference it makes.

Comment 71

[Johnathan Nightingale [:johnath]]

Attachment: Mac Current vs. w/Patch

Okay, thanks for that. I've compared before and after, here.

We need UX to make the call on the blocking-ness of the remaining changes. I agree that the patched version looks better. Is the work amenable to compartmentalization? i.e. if UX feels that the tab spacing and darker tab borders are blockers, but the curvature of the tabs isn't (or vice versa) is that an easier problem to solve, or a harder one?

Not trying to introduce added churn, trying to get clarity about how to move forward here.

UX - what's the call on blocking? All, some, or none?

Comment 72

[Stephen Horlander [:shorlander]]

(In reply to comment #71)

> UX - what's the call on blocking? All, some, or none?

It should block on all pieces.

There was a lot of design effort involved with making the tabs have clear borders/edges, distinct z-level appearance, best possible active-tab vs. inactive-tab contrast and general fit and polish.

Markus' patch addresses all of those things bringing the actual implementation inline with the desired design.

Comment 73

[Dão Gottwald [:dao]]

(In reply to comment #72)
> It should block on all pieces.

More than on Windows? I think we'll have to live with less curvature / fake overlap at the bottom there anyway, as I don't think Markus' approach would work on anything but OS X. So this requires us to maintain a completely different path for 1px more overlap -- which would be ok if it wasn't also a way more complex and scary path.

Comment 74

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

I think (correct me if I'm wrong shorlander) that shorlander is saying that the appearance blocks, not neccessarily this approach. If we can get there in ways that are easier to maintain/implement, we should do that, naturally.

Comment 75

[Stephen Horlander [:shorlander]]

(In reply to comment #74)
> I think (correct me if I'm wrong shorlander) that shorlander is saying that the
> appearance blocks, not neccessarily this approach. If we can get there in ways
> that are easier to maintain/implement, we should do that, naturally.

Correct. I am only referring to the appearance. As long as we can match what we designed I don't have any preference on implementation.

Comment 76

[Dão Gottwald [:dao]]

Sure, but one particular detail of the appearance necessarily leads to nastiness.* It won't work on Windows, I think, so I'm not sure why we would block on it on OS X.

*: See this bug's history -- we tried to find better ways but didn't get anywhere.

Comment 77

[Stephen Horlander [:shorlander]]

(In reply to comment #76)
> Sure, but one particular detail of the appearance necessarily leads to
> nastiness.* It won't work on Windows, I think, so I'm not sure why we would
> block on it on OS X.
> 
> *: See this bug's history -- we tried to find better ways but didn't get
> anywhere.

I wasn't aware that we couldn't get there on Windows. Does that mean we can't fix bug 570279?

Comment 78

[Dão Gottwald [:dao]]

There's polish pending to fix glitches, but I don't think we'll get the whole curve as intended.

Comment 79

[Markus Stange [:mstange]]

(In reply to comment #73)
> as I don't think Markus' approach would work on anything but OS X.

Maybe this is the wrong place to discuss that, but anyway: What's the specific reason that it won't work on Windows? The fact that you have to use platform colors behind the active tab? If so, couldn't we address that by using an SVG image for the tab bar instead of a PNG one? In SVG you can use platform colors.

Comment 80

[Dão Gottwald [:dao]]

Yes, I was thinking of the active tab's background. It's not a single platform color but often a hard-coded one which differs or is going to differ depending on various combinations of XP / Win 7, default / non-default OS themes, aero glass / basic and personas. The background tab color is a moving target too. So I guess this would be possible with loads of SVG images for all the combination, but that would blow up the complexity I was already concerned about.

Comment 81

[Markus Stange [:mstange]]

OK, then it won't work on Windows.

(In reply to comment #76)
> It won't work on Windows, I think, so I'm not sure why we would
> block on it on OS X.

It sounds to me like we would block on it on Windows, too, if it were possible there. The fact that it's not possible doesn't mean that we shouldn't block on it on OS X.

Comment 82

[Dão Gottwald [:dao]]

(In reply to comment #81)
> > It won't work on Windows, I think, so I'm not sure why we would
> > block on it on OS X.
> 
> It sounds to me like we would block on it on Windows, too, if it were possible
> there. The fact that it's not possible doesn't mean that we shouldn't block on
> it on OS X.

There's a subtle difference between "want really bad" and "must have". It does mean that a particular detail can't be important enough that we wouldn't ship Firefox without it. This then means that we can't take a patch at any cost.

I also brought up Windows because the ability to share the implementation across platforms could have relativized the cost (though maybe not enough). Without this the complexity is completely additive, making the Mac theme a burden.

Just as another example for why I'm concerned about this patch hindering future development, I wonder what it would mean for bug 455694. Seems like we'd get some user-facing weirdness there, unless someone comes up with a tricky way to patch up pinstripe, yet again increasing complexity. This could be a serious obstacle for people working on new features. I don't want the theme to be this much of a burden.

Comment 83

[Markus Stange [:mstange]]

(In reply to comment #82)
> Just as another example for why I'm concerned about this patch hindering future
> development, I wonder what it would mean for bug 455694.

Nothing, as far as I can tell.

> Seems like we'd get some user-facing weirdness there

What are you referring to? That small parts of the active tab curve will remain glued to the adjacent tabs while the active tab is dragged?

Comment 84

[Markus Stange [:mstange]]

Attachment: small glitch while dragging tab with the approach from bug 455694

This is what I'm referring to. Was this what you meant?

Comment 85

[Dão Gottwald [:dao]]

(In reply to comment #84)
> Created attachment 464101 [details]
> small glitch while dragging tab with the approach from bug 455694
> 
> This is what I'm referring to. Was this what you meant?

Yes, it's smaller than I imagined but still pretty ugly.

Comment 86

[Markus Stange [:mstange]]

Maybe it's time for another approach. How about this:
http://tests.themasta.com/newtrytabbar/tabdrag.xul

Downside: needs 6 additional XUL elements per tab

Upside: flawless appearance, even during animated tab dragging, and much simpler CSS code

Comment 87

[Stephen Horlander [:shorlander]]

(In reply to comment #86)
> Maybe it's time for another approach. How about this:
> http://tests.themasta.com/newtrytabbar/tabdrag.xul
> 
> Downside: needs 6 additional XUL elements per tab
> 
> Upside: flawless appearance, even during animated tab dragging, and much
> simpler CSS code

That looks amazing! Is that something we could do on Windows and Linux as well? What would be the problem with the 6 additional XUL elements? Performance?

Comment 88

[Dão Gottwald [:dao]]

(In reply to comment #86)
> Downside: needs 6 additional XUL elements per tab
> 
> Upside: flawless appearance, even during animated tab dragging, and much
> simpler CSS code

Sounds like a fair tradeoff. I suppose an SVG mask can't have flexible and fixed parts à la border image?

Comment 89

[Stephen Horlander [:shorlander]]

According to the MDC -moz-border-image docs you can use an SVG image but it doesn't give an example of how to do so.

https://developer.mozilla.org/En/CSS/-moz-border-image#Values

Comment 90

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You can't use an SVG image. First dholbert's patches have to landed (bug 231179) and then some extra work would be required to make them work with border-image.

Comment 91

[Markus Stange [:mstange]]

(In reply to comment #89)
> According to the MDC -moz-border-image docs you can use an SVG image

The problem is something else: Whether you can use something *in SVG* that works like border-image; specifically, whether it's possible to create an SVG mask that has both absolute and relative components. As far as I can tell this is not possible. It's either "objectBoundingBox" or "userSpaceOnUse": With "objectBoundingBox" you can make a mask that adapts to the masked element's size, and with "userSpaceOnUse" you can use absolute pixel values. But for example a mask that simulates -moz-border-radius:5px, i.e. adapts to the masked element's size while keeping a fixed radius, doesn't seem possible.
I tried using a foreignObject inside the mask (which maybe could contain a div that border-image could be used on), but it seemed to be ignored.

Comment 92

[Markus Stange [:mstange]]

(In reply to comment #87)
> Is that something we could do on Windows and Linux as well?

Yes, I think we could.

> What would be the problem with the 6 additional XUL elements? Performance?

Untidy code, mostly. But if Dão's fine with it I won't say anything :)

As far as performance is concerned the bigger risk lies in the use of SVG masks.

Comment 93

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #91)
> The problem is something else: Whether you can use something *in SVG* that
> works like border-image; specifically, whether it's possible to create an SVG
> mask that has both absolute and relative components. As far as I can tell this
> is not possible. It's either "objectBoundingBox" or "userSpaceOnUse": With
> "objectBoundingBox" you can make a mask that adapts to the masked element's
> size, and with "userSpaceOnUse" you can use absolute pixel values. But for
> example a mask that simulates -moz-border-radius:5px, i.e. adapts to the masked
> element's size while keeping a fixed radius, doesn't seem possible.
> I tried using a foreignObject inside the mask (which maybe could contain a div
> that border-image could be used on), but it seemed to be ignored.

It actually is possible, by nesting SVG elements. E.g.
  <svg>
    <g x="100%" y="100%">
      <circle cx="-5" cy="-5" r="5"/>
    </g>
  </svg>
puts a circle in the bottom right corner of the SVG element.

However, using multiple XUL elements is definitely going to perform better than using a complex SVG image or mask.

Comment 94

[Markus Stange [:mstange]]

(In reply to comment #93)
> It actually is possible, by nesting SVG elements. E.g.
>   <svg>
>     <g x="100%" y="100%">
>       <circle cx="-5" cy="-5" r="5"/>
>     </g>
>   </svg>
> puts a circle in the bottom right corner of the SVG element.

There you're using percentages, and those are relative to the <svg> element's size, not to the size of the element that the mask is applied to.

> However, using multiple XUL elements is definitely going to perform better than
> using a complex SVG image or mask.

OK.

Comment 95

[Dão Gottwald [:dao]]

(In reply to comment #93)
> However, using multiple XUL elements is definitely going to perform better than
> using a complex SVG image or mask.

What does qualify as complex? Note that http://tests.themasta.com/newtrytabbar/tabdrag.xul uses two SVG masks already.

Comment 96

[Robert O'Callahan (:roc) (email my personal email if necessary)]

True.

BTW, if you can use clip-path instead of mask, clip-path is probably faster. Although you should test all this.

Comment 97

[Shawn Wilsher :sdwilsh]

(In reply to comment #68)
> I'm quite upset that this has gone several days without a comment; I assume
> that means it needs to move to beta4.
> 
> I expect this to land *EARLY* in the beta4 cycle.
It clearly didn't land early in the beta4 cycle since we are very close to freeze on that.  This should only need beta coverage and is not a regression from previous betas, so moving to blocking betaN.

Comment 98

[Markus Stange [:mstange]]

Attachment: v9

Comment 99

[Markus Stange [:mstange]]

Attachment: v10

on top of the new patch in bug 593967 and without dependency on bug 547805

Comment 100

[Markus Stange [:mstange]]

Attachment: v11

simplified two selectors

Comment 101

[Stephen Horlander [:shorlander]]

Attachment: v11 visual strangeness

I am rebuilding now to see if it isn't something else but this is what I get when I apply v11.

Comment 102

[Markus Stange [:mstange]]

You'll also need to apply the patches from bug 587585 and bug 594002.

Comment 103

[Stephen Horlander [:shorlander]]

(In reply to comment #102)
> You'll also need to apply the patches from bug 587585 and bug 594002.

Thanks! That fixed it. Now looks amazing: http://grab.by/6p2N :)

Comment 104

[Markus Stange [:mstange]]

Attachment: v12

with .arrowscrollbox-scrollbox instead of .scrollbox and updated to trunk

Comment 105

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

Setting to beta8+ so we can make sure it gets done earlier rather than later.

Comment 106

[Markus Stange [:mstange]]

Attachment: v13

updated to trunk

Comment 107

[Arne Stenström]

https://bug547787.bugzilla.mozilla.org/attachment.cgi?id=462414

(comment #103)
> Now looks amazing: http://grab.by/6p2N :)

The contrast between the active tab and other tabs is rather low with tabs-on-bottom. Is this how it will be?

Comment 108

[Caspy7]

(In reply to comment #107)
> The contrast between the active tab and other tabs is rather low with
> tabs-on-bottom. Is this how it will be?

I second this sentiment.  The low contrast for tabs on bottom is a concern in my mind.  Is there any way we can remedy this?

Comment 109

[Dão Gottwald [:dao]]

Comment on attachment 485503 [details] [diff] [review]
v13

Can you make the numerous "#TabsToolbar[tabsontop=...] .tab-foo" selectors more efficient? For starters, you should be using #tabbrowser-tabs[tabsontop=...], and then I'd suggest defining preprocessing shorthands for "#tabbrowser-tabs[tabsontop=true/false] > .tabbrowser-tab > .tab-stack >".

Comment 110

[Markus Stange [:mstange]]

Attachment: v14

Done. I kept the trailing ">" outside the macro so that it's obvious which kind of selector is being used without having to look at the shorthand definition.

I've made two other changes in this patch: I've put the dropmarker on a higher z level than the selected tab, and I've changed the selector of the "apply position:relative to selected tabs" rule to also include pinned tabs in a pinnedonly tabbox, since those aren't position:fixed.

Comment 111

[Markus Stange [:mstange]]

I've pushed both v13 and v14 to the tryserver in order to see whether optimizing the selectors made any difference in our performance tests. It didn't:
> http://perf.snarkfest.net/compare-talos/index.html?oldRevs=39d58fc46911&newRev=3136bc6eff99&tests=%20a11y,tdhtml,tdhtml_nochrome,tp4,tp4_memset,tp4_pbytes,tp4_rss,tp4_shutdown,tp4_xres,dromaeo_basics,dromaeo_css,dromaeo_dom,dromaeo_jslib,dromaeo_sunspider,dromaeo_v8,tsspider,tsspider_nochrome,tgfx,tgfx_nochrome,tscroll,tsvg,tsvg_opacity,ts,ts_cold,ts_cold_generated_max,ts_cold_generated_max_shutdown,ts_cold_generated_med,ts_cold_generated_med_shutdown,ts_cold_generated_min,ts_cold_generated_min_shutdown,ts_cold_shutdown,ts_places_generated_max,ts_places_generated_max_shutdown,ts_places_generated_med,ts_places_generated_med_shutdown,ts_places_generated_min,ts_places_generated_min_shutdown,ts_shutdown,twinopen&submit=true

Comment 112

[Dão Gottwald [:dao]]

Comment on attachment 492951 [details] [diff] [review]
v14

>+#tabbrowser-tabs[pinnedonly="true"] .tabbrowser-tab[selected="true"] {

use the child selector

Can background tabs have background colors for :-moz-lwtheme-brighttext and :-moz-lwtheme-darktext à la winstripe / gnomestripe? Right now background tabs tend to fade away since they are very transparent, and there's hardly any difference between background tabs and the active tab with tabs on bottom.

Comment 113

[Dão Gottwald [:dao]]

Comment on attachment 492951 [details] [diff] [review]
v14

>+@TABSONTOP_TAB@:hover > * > .tab-content:not([selected="true"]),

>+@TABSONBOTTOM_TAB@:hover > * > .tab-content:not([selected="true"]),

Please spell out .tab-stack here. Makes it easier to maintain this should we ever touch the anonymous content's structure again. We should only resort to the asterisk when selectors become unbearably long.

Comment 114

[Markus Stange [:mstange]]

(In reply to comment #112)
> Can background tabs have background colors for :-moz-lwtheme-brighttext and
> :-moz-lwtheme-darktext à la winstripe / gnomestripe?

I added some (copied from winstripe). The big problem with this is that we can't apply them to the new tab button, because that doesn't have the rounded mask, so it looks a little inconsistent now.

> there's hardly any
> difference between background tabs and the active tab with tabs on bottom.

If we had bug 547805, fixing this would be easier, since we could just give the tabbar a darker background which would shine through the background tabs. I take it it's too late for that to get into Firefox 4?

(In reply to comment #113)
> Please spell out .tab-stack here. Makes it easier to maintain this should we
> ever touch the anonymous content's structure again.

Good point.

I also noticed that the titlechanged styling was messed up (three white dots per tab instead of one) and fixed it.

Comment 115

[Markus Stange [:mstange]]

Attachment: v15

Comment 116

[Markus Stange [:mstange]]

(In reply to comment #114)
> The big problem with this is that we
> can't apply them to the new tab button, because that doesn't have the rounded
> mask

I could add another mask that exactly fits on the new tab button, and then set that mask and the background on the .toolbarbutton-icon in the button. Should I do that?

Comment 117

[Dão Gottwald [:dao]]

(In reply to comment #116)
> I could add another mask that exactly fits on the new tab button, and then set
> that mask and the background on the .toolbarbutton-icon in the button. Should I
> do that?

I'd leave that to a separate bug.

Can you attach a screenshot of the lightweight theme styling? Tabs look kind of broken over here, but that might be because I'm testing this on Windows.

Comment 118

[Markus Stange [:mstange]]

Attachment: lwtheme screenshots

I wouldn't disagree if you called this broken... :)
We can always tweak it later.

Comment 119

[Dão Gottwald [:dao]]

Comment on attachment 493230 [details] [diff] [review]
v15

>+@TABSONTOP_TAB_STACK@ > .tab-background:not([selected="true"]) {
>+  margin-bottom: 2px;

>+#TabsToolbar[tabsontop="true"] {
>+  padding-bottom: 2px;

>+#tabbrowser-tabs[tabsontop="true"] > .tabbrowser-arrowscrollbox > .arrowscrollbox-scrollbox {
>+  margin-bottom: -2px;

Adding :not(:-moz-lwtheme) to these selectors should help a bit.

Comment 120

[Markus Stange [:mstange]]

Attachment: v16

Looks better than I expected!

Comment 121

[Dão Gottwald [:dao]]

Comment on attachment 493380 [details] [diff] [review]
v16

>+ * Make sure that the width animation can animate all the way down to zero.

Is this still needed? I landed http://hg.mozilla.org/mozilla-central/rev/ff7070b83964 today, hiding the tab when the width nears zero.

Comment 122

[Markus Stange [:mstange]]

Attachment: v17

Looks like it's not needed anymore, cool. Removing it does make the jump that occurs when closing the rightmost tab in overflow mode a little worse, but I think that's a scrollbox bug.

Comment 123

[Dão Gottwald [:dao]]

Comment on attachment 493401 [details] [diff] [review]
v17

>+    <svg:mask id="tab-ontop-left-curve-mask" maskContentUnits="userSpaceOnUse">
>+      <svg:circle cx="9" cy="3" r="3" fill="white"/>
>+      <svg:rect x="9" y="0" width="3" height="3" fill="white"/>
>+      <svg:rect x="6" y="3" width="6" height="19" fill="white"/>
>+      <svg:rect x="1" y="17" width="5" height="5" fill="white"/>
>+      <svg:circle cx="1" cy="17" r="5"/>
>+      <svg:rect x="0" y="22" width="12" height="1" fill="white"/>
>+    </svg:mask>
>+    <svg:mask id="tab-ontop-right-curve-mask" maskContentUnits="userSpaceOnUse">
>+      <svg:circle cx="3" cy="3" r="3" fill="white"/>
>+      <svg:rect x="0" y="0" width="3" height="3" fill="white"/>
>+      <svg:rect x="0" y="3" width="6" height="19" fill="white"/>
>+      <svg:rect x="6" y="17" width="5" height="5" fill="white"/>
>+      <svg:circle cx="11" cy="17" r="5"/>
>+      <svg:rect x="0" y="22" width="12" height="1" fill="white"/>
>+    </svg:mask>
>+    <svg:mask id="tab-onbottom-left-curve-mask" maskContentUnits="userSpaceOnUse">
>+      <svg:circle cx="9" cy="20" r="3" fill="white"/>
>+      <svg:rect x="9" y="20" width="3" height="3" fill="white"/>
>+      <svg:rect x="6" y="1" width="6" height="19" fill="white"/>
>+      <svg:rect x="1" y="1" width="5" height="5" fill="white"/>
>+      <svg:circle cx="1" cy="6" r="5"/>
>+      <svg:rect x="0" y="0" width="12" height="1" fill="white"/>
>+    </svg:mask>
>+    <svg:mask id="tab-onbottom-right-curve-mask" maskContentUnits="userSpaceOnUse">
>+      <svg:circle cx="3" cy="20" r="3" fill="white"/>
>+      <svg:rect x="0" y="20" width="3" height="3" fill="white"/>
>+      <svg:rect x="0" y="1" width="6" height="19" fill="white"/>
>+      <svg:rect x="6" y="1" width="5" height="5" fill="white"/>
>+      <svg:circle cx="11" cy="6" r="5"/>
>+      <svg:rect x="0" y="0" width="12" height="1" fill="white"/>
>+    </svg:mask>
>+  </svg:svg>

Prefix the ids with "pinstripe-", as this is a hack and we don't want anyone else to start depending on these nodes existing in browser.xul.

>+        <xul:hbox xbl:inherits="fadein,pinned,selected,titlechanged"
>+                  class="tab-background">
>+          <xul:hbox xbl:inherits="fadein,pinned,selected,titlechanged"
>+                    class="tab-background-start"/>
>+          <xul:hbox xbl:inherits="fadein,pinned,selected,titlechanged"
>+                    class="tab-background-middle"/>
>+          <xul:hbox xbl:inherits="fadein,pinned,selected,titlechanged"
>+                    class="tab-background-end"/>
>+        </xul:hbox>
>+        <xul:hbox xbl:inherits="fadein,pinned,selected,titlechanged"
>+                  class="tab-content" align="center">

Don't inherit 'fadein'.

> .tabbrowser-tab,
> .tabs-newtab-button {

>+  border: 0 solid transparent;

border: none;?

> #TabsToolbar[tabsontop="true"]:not(:-moz-lwtheme) {
>-  padding-bottom: 1px;
>-  box-shadow: 0 -6px 3.5px -5px rgba(0,0,0,.3) inset,
>-              0 -2px 0 rgba(0,0,0,.2) inset;
>+  padding-bottom: 2px;
> }

>+#TabsToolbar[tabsontop="true"]:not(:-moz-lwtheme) {
>+  background-image: url(chrome://browser/skin/tabbrowser/tabbar-top-bg-active.png) ;
>+}

Combine these two rules.

Comment 124

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/a5500a756268

Comment 125

[Thomas Stache]

Attachment: Screenshot showing glitches in the curves with personas

This seems to have rendering glitches in the curves connecting the tabs to the next toolbar when Personas are in use. Is there already a followup bug on file?

Comment 126

[Jorge R]

Using personas there are some problems with the rounded borders of the tabs, you can see the border of the other tabs overlapping active tab edges.