Closed
Bug 547787
Opened 14 years ago
Closed 14 years ago
New style for the tab bar
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 4.0b8
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta8+ |
People
(Reporter: mstange, Assigned: mstange)
References
Details
Attachments
(6 files, 26 obsolete files)
No description provided.
Comment 1•14 years ago
|
||
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...
Assignee | ||
Comment 2•14 years ago
|
||
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•14 years ago
|
||
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.
Assignee | ||
Comment 4•14 years ago
|
||
I need to think about that a little. Tryserver build is here: https://build.mozilla.org/tryserver-builds/mstange@themasta.com-try-960e374df989/try-960e374df989-macosx.dmg
Comment 5•14 years ago
|
||
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.
Assignee | ||
Comment 6•14 years ago
|
||
(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•14 years ago
|
||
(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.
Assignee | ||
Comment 8•14 years ago
|
||
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.
Assignee | ||
Comment 9•14 years ago
|
||
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".
Attachment #428259 -
Attachment is obsolete: true
Assignee | ||
Comment 10•14 years ago
|
||
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.
Attachment #429322 -
Attachment is obsolete: true
Assignee | ||
Comment 11•14 years ago
|
||
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/mstange@themasta.com-try-aec49c7a0c36/try-aec49c7a0c36-macosx.dmg
Attachment #430590 -
Attachment is obsolete: true
Assignee | ||
Comment 12•14 years ago
|
||
Attachment #431091 -
Attachment is obsolete: true
Attachment #438734 -
Flags: review?(dao)
Assignee | ||
Comment 13•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
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.
Attachment #438734 -
Flags: review?(dao) → review-
Assignee | ||
Comment 16•14 years ago
|
||
(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•14 years ago
|
||
(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.
Assignee | ||
Comment 18•14 years ago
|
||
(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•14 years ago
|
||
(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.
Assignee | ||
Comment 20•14 years ago
|
||
(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•14 years ago
|
||
(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...
Assignee | ||
Comment 22•14 years ago
|
||
(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.
Assignee | ||
Comment 23•14 years ago
|
||
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.
Assignee | ||
Comment 25•14 years ago
|
||
Dão, I'm waiting for an answer to comment 23 before continuing.
Comment 26•14 years ago
|
||
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.
Assignee | ||
Comment 27•14 years ago
|
||
(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•14 years ago
|
||
(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".
Assignee | ||
Comment 29•14 years ago
|
||
(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•14 years ago
|
||
(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.
Assignee | ||
Comment 31•14 years ago
|
||
(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.
Assignee | ||
Comment 32•14 years ago
|
||
(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.
Assignee | ||
Comment 33•14 years ago
|
||
This is a patch that still uses ::before and -moz-image-rect but fixes all the problems you and Stephen noticed with v1.
Attachment #438734 -
Attachment is obsolete: true
Attachment #442151 -
Flags: review?(dao)
Assignee | ||
Comment 34•14 years ago
|
||
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.
Assignee | ||
Comment 35•14 years ago
|
||
There was a small mistake in the last patch.
Attachment #442153 -
Attachment is obsolete: true
Assignee | ||
Comment 36•14 years ago
|
||
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.
Attachment #442151 -
Attachment is obsolete: true
Attachment #442472 -
Flags: review?(dao)
Attachment #442151 -
Flags: review?(dao)
Comment 37•14 years ago
|
||
(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? :)
Assignee | ||
Comment 38•14 years ago
|
||
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•14 years ago
|
||
(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.
Assignee | ||
Comment 40•14 years ago
|
||
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•14 years ago
|
||
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?
Assignee | ||
Comment 42•14 years ago
|
||
(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.
Assignee | ||
Comment 43•14 years ago
|
||
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.
Attachment #442472 -
Flags: review?(dao)
Comment 44•14 years ago
|
||
(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.
Assignee | ||
Comment 45•14 years ago
|
||
with those changes, and I also added some documentation
Attachment #442472 -
Attachment is obsolete: true
Attachment #447528 -
Flags: review?(dao)
Assignee | ||
Comment 46•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
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?
Assignee | ||
Comment 49•14 years ago
|
||
(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•14 years ago
|
||
(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.
Assignee | ||
Comment 51•14 years ago
|
||
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.
Assignee | ||
Comment 52•14 years ago
|
||
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?
Attachment #447528 -
Attachment is obsolete: true
Attachment #452215 -
Flags: review?(dao)
Attachment #447528 -
Flags: review?(dao)
Assignee | ||
Comment 53•14 years ago
|
||
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.
Attachment #447533 -
Attachment is obsolete: true
Assignee | ||
Comment 54•14 years ago
|
||
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.
Attachment #452215 -
Flags: review?(dao) → feedback?(dao)
Assignee | ||
Comment 55•14 years ago
|
||
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.
Attachment #452215 -
Attachment is obsolete: true
Attachment #453123 -
Flags: review?(dao)
Attachment #452215 -
Flags: feedback?(dao)
Assignee | ||
Comment 56•14 years ago
|
||
The generator was broken in the previous patch.
Attachment #453123 -
Attachment is obsolete: true
Attachment #453162 -
Flags: review?(dao)
Attachment #453123 -
Flags: review?(dao)
Comment 57•14 years ago
|
||
While it's not a blocker, could this make it into Beta 1 for testing?
Assignee | ||
Comment 58•14 years ago
|
||
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•14 years ago
|
||
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.
blocking2.0: --- → beta2+
Comment 60•14 years ago
|
||
Comment on attachment 453162 [details] [diff] [review] v7 This doesn't seem to work for app tabs ... even worse, I don't see why.
Attachment #453162 -
Flags: review?(dao) → review-
Assignee | ||
Comment 61•14 years ago
|
||
Looks like position:fixed is at fault again. I'll see what I can do.
Assignee | ||
Comment 62•14 years ago
|
||
The patch in bug 579776 fixes the app tab problem.
Assignee | ||
Comment 64•14 years ago
|
||
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.
Attachment #453162 -
Attachment is obsolete: true
Attachment #459128 -
Flags: review?(dao)
Assignee | ||
Comment 65•14 years ago
|
||
Tryserver build: http://stage.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/mstange@themasta.com-72e2ce422690/tryserver-macosx/firefox-4.0b3pre.en-US.mac.dmg
Comment 67•14 years ago
|
||
Dao, will this make it for b3 code freeze on Monday, Aug 2?
Comment 68•14 years ago
|
||
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.
blocking2.0: beta3+ → beta4+
Comment 69•14 years ago
|
||
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.
Assignee | ||
Comment 70•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
There's polish pending to fix glitches, but I don't think we'll get the whole curve as intended.
Assignee | ||
Comment 79•14 years ago
|
||
(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•14 years ago
|
||
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.
Assignee | ||
Comment 81•14 years ago
|
||
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•14 years ago
|
||
(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.
Assignee | ||
Comment 83•14 years ago
|
||
(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?
Assignee | ||
Comment 84•14 years ago
|
||
This is what I'm referring to. Was this what you meant?
Comment 85•14 years ago
|
||
(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.
Assignee | ||
Comment 86•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
(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•14 years ago
|
||
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
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.
Assignee | ||
Comment 91•14 years ago
|
||
(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.
Assignee | ||
Comment 92•14 years ago
|
||
(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.
(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.
Assignee | ||
Comment 94•14 years ago
|
||
(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•14 years ago
|
||
(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.
True. BTW, if you can use clip-path instead of mask, clip-path is probably faster. Although you should test all this.
Updated•14 years ago
|
Attachment #459128 -
Flags: review?(dao)
Updated•14 years ago
|
Attachment #442167 -
Attachment is obsolete: true
Updated•14 years ago
|
Attachment #440516 -
Attachment is obsolete: true
Updated•14 years ago
|
Attachment #439251 -
Attachment is obsolete: true
Comment 97•14 years ago
|
||
(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.
blocking2.0: beta4+ → betaN+
Assignee | ||
Comment 98•14 years ago
|
||
Attachment #459128 -
Attachment is obsolete: true
Attachment #473796 -
Flags: review?(dao)
Assignee | ||
Comment 99•14 years ago
|
||
on top of the new patch in bug 593967 and without dependency on bug 547805
Attachment #473796 -
Attachment is obsolete: true
Attachment #475470 -
Flags: review?(dao)
Attachment #473796 -
Flags: review?(dao)
Assignee | ||
Comment 100•14 years ago
|
||
simplified two selectors
Attachment #475470 -
Attachment is obsolete: true
Attachment #475491 -
Flags: review?(dao)
Attachment #475470 -
Flags: review?(dao)
Comment 101•14 years ago
|
||
I am rebuilding now to see if it isn't something else but this is what I get when I apply v11.
Assignee | ||
Comment 102•14 years ago
|
||
You'll also need to apply the patches from bug 587585 and bug 594002.
Comment 103•14 years ago
|
||
(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 :)
Assignee | ||
Comment 104•14 years ago
|
||
with .arrowscrollbox-scrollbox instead of .scrollbox and updated to trunk
Attachment #475491 -
Attachment is obsolete: true
Attachment #475549 -
Attachment is obsolete: true
Attachment #477304 -
Flags: review?(dao)
Attachment #475491 -
Flags: review?(dao)
Comment 105•14 years ago
|
||
Setting to beta8+ so we can make sure it gets done earlier rather than later.
blocking2.0: betaN+ → beta8+
Assignee | ||
Comment 106•14 years ago
|
||
updated to trunk
Attachment #477304 -
Attachment is obsolete: true
Attachment #485503 -
Flags: review?(dao)
Attachment #477304 -
Flags: review?(dao)
Comment 107•14 years ago
|
||
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•14 years ago
|
||
(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•14 years ago
|
||
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 >".
Assignee | ||
Comment 110•14 years ago
|
||
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.
Attachment #485503 -
Attachment is obsolete: true
Attachment #492951 -
Flags: review?(dao)
Attachment #485503 -
Flags: review?(dao)
Assignee | ||
Comment 111•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
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.
Assignee | ||
Comment 114•14 years ago
|
||
(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.
Assignee | ||
Comment 115•14 years ago
|
||
Attachment #492951 -
Attachment is obsolete: true
Attachment #493230 -
Flags: review?(dao)
Attachment #492951 -
Flags: review?(dao)
Assignee | ||
Comment 116•14 years ago
|
||
(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•14 years ago
|
||
(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.
Assignee | ||
Comment 118•14 years ago
|
||
I wouldn't disagree if you called this broken... :) We can always tweak it later.
Comment 119•14 years ago
|
||
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.
Assignee | ||
Comment 120•14 years ago
|
||
Looks better than I expected!
Attachment #493230 -
Attachment is obsolete: true
Attachment #493380 -
Flags: review?(dao)
Attachment #493230 -
Flags: review?(dao)
Comment 121•14 years ago
|
||
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.
Assignee | ||
Comment 122•14 years ago
|
||
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.
Attachment #493380 -
Attachment is obsolete: true
Attachment #493401 -
Flags: review?(dao)
Attachment #493380 -
Flags: review?(dao)
Comment 123•14 years ago
|
||
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.
Attachment #493401 -
Flags: review?(dao) → review+
Assignee | ||
Comment 124•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a5500a756268
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
Comment 125•14 years ago
|
||
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•14 years ago
|
||
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.
Updated•14 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•