348967 - Tab strip gets taller when scroll buttons appear

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Peter Kasting]

After the landing of bug 345466, the tab strip gets taller when the scroll buttons appear.  I saw this on my Linux box earlier tonight and don't have a branch build here on Windows to test, so no screen shots from me at the moment.

Comment 1

[Dão Gottwald [:dao]]

(In reply to comment #0)
> After the landing of bug 345466, ...

I've seen this before. See attachment 228917 [details] / bug 344155 comment 2.

Comment 2

[Peter Kasting]

Attachment: No scroll buttons vs. scroll buttons

Here's a composite screenshot on my Linux box.  The height difference is 2 px.

Comment 3

[Adam Guthrie]

See also bug 345384.

Comment 4

[(not reading, please use [e-mail] instead)]

changing the style of the three elements in the alltabs stack to include "margin-bottom: 1px ! important; height: 26px ! important;" fixes this bug, and bug #348984

here comes a patch....

Comment 5

[(not reading, please use [e-mail] instead)]

> "margin-bottom: 1px ! important; height: 26px ! important;" fixes this bug

actually, I think the right fix is to style elements in the alltabs stack similar to how the scrollbuttons are styled, and that would be to just add margin-bottom: 2px (and leave the height at 25 px)

Comment 6

[(not reading, please use [e-mail] instead)]

Attachment: a hand made patch, with just the changes for this bug (my browser.css has too many fixes in it)

Comment 7

[(not reading, please use [e-mail] instead)]

Attachment: screen shot after the patch

Comment 8

[(not reading, please use [e-mail] instead)]

mconnor:  sorry for the hand made patch, but my browser.css is quickly becoming poluted with several theme fixes.

here's the idea behind the fix.  The scrollup and scrolldown buttons have 2px margin bottom and 2px margin top.

I've made it so the 3 elements in the all tabs stack have these same margins, and the addition two elements in the scrolldown stack have the same margins, too.

I'll pull another tree and get you a clean patch with just this fix, if you need it.

Comment 9

[(not reading, please use [e-mail] instead)]

Attachment: a real patch, from a clean tree

also seeking beltzner's feedback, should the gap be 2px (all the time) or 1px (all the time?)

Comment 10

[Peter Kasting]

(In reply to comment #9)
> also seeking beltzner's feedback, should the gap be 2px (all the time) or 1px
> (all the time?)

Is this the cause of the difference in this screenshot: https://bugzilla.mozilla.org/attachment.cgi?id=234398 ?
If so, then please oh please oh please make it 1 px.

Comment 11

[(not reading, please use [e-mail] instead)]

> Is this the cause of the difference in this screenshot:
> https://bugzilla.mozilla.org/attachment.cgi?id=234398 ?
> If so, then please oh please oh please make it 1 px.

peter, which bug is that from?

If we have a bug about changing that from 2px to 1px (already approved by mconnor or beltzner), that would help.

Comment 12

[Peter Kasting]

(In reply to comment #11)
> > Is this the cause of the difference in this screenshot:
> > https://bugzilla.mozilla.org/attachment.cgi?id=234398 ?
> > If so, then please oh please oh please make it 1 px.
> 
> peter, which bug is that from?

This is the screenshot I emailed you earlier today.  It's part of bug 348935.  You should also see bug 348983, which I think is already assigned to you.

Comment 13

[(not reading, please use [e-mail] instead)]

there's a bigger problem with tabs vs the alltabs button vs the scrollbuttons that I'm finally noticing as I spend all day looking at dimensions, borders, margins and background images.

at least for winstripe, the tab strip is 29 px tall, and the tabs are 28 px tall (with a 1px margin on the bottom), but inside the tab, the elements are 24 px tall (as are the background images we use for these elements)

the alltabs button and the scrollbutton-down stacks are 29 px tall, but the inner elements are 25 px, as are the images.

In order to solve these bugs, I need to make the scrollbuttons and the all tabs button have the same sized background images, and have similar dimensions to the tabs.

mconnor also recently talked to me about using min-height (instead of height) and larger background images that would scale nicely when the dpi changes to 200%.

Comment 14

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

> also seeking beltzner's feedback, should the gap be 2px (all the time) or 1px
> (all the time?)

In Fx 1.5 on Mac, we had a 1px gap at the top.
In Fx 1.5 on W32, we had a 2px gap at the top, but the selected tab was 1px taller than the others.
I don't have a Linux copy around to check, but I'm guessing it was the same as w32.

For Firefox 2, on all platforms, it is my assertion and strong opinion that we should:

 - leave a 2px gap at the top
 - make the selected tab 1px "taller" than the others

Comment 15

[Peter Kasting]

(In reply to comment #14)
> For Firefox 2, on all platforms, it is my assertion and strong opinion that we
> should:
> 
>  - leave a 2px gap at the top
>  - make the selected tab 1px "taller" than the others

OK; that's bug 349187, so requesting blocking on that, and setting this bug to block that one.

Comment 16

[Peter Kasting]

(In reply to comment #14)
> In Fx 1.5 on W32, we had a 2px gap at the top, but the selected tab was 1px
> taller than the others.
> I don't have a Linux copy around to check, but I'm guessing it was the same as
> w32.

On my Linux box, unselected tabs have a 3px gap above them, and selected tabs have a 1px gap.

Comment 17

[(not reading, please use [e-mail] instead)]

Comment on attachment 235327 [details] [diff] [review]
a real patch, from a clean tree

having 2px has been agreed to by beltzner.

Comment 18

[Peter Kasting]

(In reply to comment #17)
> (From update of attachment 235327 [details] [diff] [review] [edit])
> having 2px has been agreed to by beltzner.

beltzner wanted 2px on top, right?  What about on the bottom?  I thought in comment 11 you implied the bottom should be 1 px.

Comment 19

[(not reading, please use [e-mail] instead)]

> beltzner wanted 2px on top, right?  

right, 2px on top, for non-selected tabs, and 1px for the selected tab.

the patch in this bug, plus the patch in bug #349187 will accomplish this for winstripe.

> What about on the bottom?  I thought in comment 11 you implied 
> the bottom should be 1 px.

I didn't intend to imply anything about the bottom.  Can you elaborate?

Comment 20

[(not reading, please use [e-mail] instead)]

a collow up to comment #14

<sspitzer> in 2.0, are we going to make the selected tab taller, and give it the focused look?
<beltzner> I think we should, yes
<beltzner> but not the focused look
<beltzner> with the yellow highlight
<beltzner> just bigger and brighter than the others, I think

Comment 21

[Peter Kasting]

(In reply to comment #19)
> > What about on the bottom?  I thought in comment 11 you implied 
> > the bottom should be 1 px.
> 
> I didn't intend to imply anything about the bottom.  Can you elaborate?

In comment 8 you said:
"The scrollup and scrolldown buttons have 2px margin bottom..."

In comment 10, referring to the margin on the bottom, I said:
"(In reply to comment #9)
> also seeking beltzner's feedback, should the gap be 2px (all the time) or 1px
> (all the time?)

Is this the cause of the difference in this screenshot:
https://bugzilla.mozilla.org/attachment.cgi?id=234398 ?
If so, then please oh please oh please make it 1 px."

In comment 11 you responded, presumably still about the bottom margin:
"If we have a bug about changing that from 2px to 1px (already approved by
mconnor or beltzner)..."

But in your patch ( https://bugzilla.mozilla.org/attachment.cgi?id=235327 ), the only changes are to add 2px bottom margins.  Yet your comment 17 said:
"having 2px has been agreed to by beltzner."

So I am confused.  Everyone agrees 2px on top is good, and that's what you claim the patch does, but that's not what it looks like it does.  I'm not sure whether 2px on bottom is good at all, but that's not what your patch seems to change.

Comment 22

[(not reading, please use [e-mail] instead)]

I think part of the confusion is coming from screen shots which show multiple issues at once (as we have multiple issues which need fixing.)

in order to accomplish the goal of having 2px above the scrollbuttons, the alltabs button, and the tabs (except for the selected tab, see bug #349187), the patch I have attached (that adds "margin-bottom: 2px;" to various elements, to get them in sync), is needed.

there are lots of things going on in various bugs about the tab strip.  let's take the conversations about what's going on under the tabs, or at the bottom of the tabs, to another bug, like #348935 or bug #348983.	

Comment 23

[Mike Connor [:mconnor]]

Comment on attachment 235327 [details] [diff] [review]
a real patch, from a clean tree

r+a=me, sorry about the delay here

Comment 24

[(not reading, please use [e-mail] instead)]

fix checked into the branch for winstripe, pinstripe still needs fixing.

Comment 25

[(not reading, please use [e-mail] instead)]

fixed on pinstripe, too (with the landing of the images and css from bug
#350139).