Tab strip gets taller when scroll buttons appear

RESOLVED FIXED in Firefox 2

Status

()

Firefox
Tabbed Browser
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Peter Kasting, Assigned: (not reading, please use seth@sspitzer.org instead))

Tracking

({fixed1.8.1})

2.0 Branch
Firefox 2
fixed1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Fx2 theme change])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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.
(Reporter)

Updated

12 years ago
Flags: blocking-firefox2?
Target Milestone: --- → Firefox 2

Comment 1

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

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

Comment 2

12 years ago
Created attachment 234239 [details]
No scroll buttons vs. scroll buttons

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

Updated

12 years ago
Flags: blocking-firefox2? → blocking-firefox2+

Comment 3

12 years ago
See also bug 345384.

Updated

12 years ago
Whiteboard: [Fx2 theme change]

Updated

12 years ago
Assignee: nobody → sspitzer
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....
Blocks: 348984
Status: NEW → ASSIGNED
> "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)
Created attachment 235301 [details]
a hand made patch, with just the changes for this bug (my browser.css has too many fixes in it)
Attachment #235301 - Flags: review?(mconnor)
Created attachment 235302 [details]
screen shot after the patch
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.
Created attachment 235327 [details] [diff] [review]
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?)
Attachment #235301 - Attachment is obsolete: true
Attachment #235327 - Flags: ui-review?(beltzner)
Attachment #235327 - Flags: review?(mconnor)
Attachment #235301 - Flags: review?(mconnor)
(Reporter)

Comment 10

12 years ago
(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.
> 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.
(Reporter)

Comment 12

12 years ago
(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.
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%.
> 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
(Reporter)

Comment 15

12 years ago
(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.
Blocks: 349187
(Reporter)

Comment 16

12 years ago
(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 on attachment 235327 [details] [diff] [review]
a real patch, from a clean tree

having 2px has been agreed to by beltzner.
Attachment #235327 - Flags: ui-review?(beltzner) → ui-review+
Whiteboard: [Fx2 theme change] → [Fx2 theme change][seeking reviews mconnor]
(Reporter)

Comment 18

12 years ago
(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.
> 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?
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
(Reporter)

Comment 21

12 years ago
(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.
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 on attachment 235327 [details] [diff] [review]
a real patch, from a clean tree

r+a=me, sorry about the delay here
Attachment #235327 - Flags: review?(mconnor)
Attachment #235327 - Flags: review+
Attachment #235327 - Flags: approval1.8.1+
fix checked into the branch for winstripe, pinstripe still needs fixing.
OS: Linux → All
Hardware: PC → All
Whiteboard: [Fx2 theme change][seeking reviews mconnor] → [Fx2 theme change][fixed on winstripe, still need to fix pinstripe]
fixed on pinstripe, too (with the landing of the images and css from bug
#350139).
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Depends on: 350139
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [Fx2 theme change][fixed on winstripe, still need to fix pinstripe] → [Fx2 theme change]
You need to log in before you can comment on or make changes to this bug.