482965 - tabbrowser-tabs shouldn't have horizontal margin

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Dão Gottwald [:dao]]

STR:
1. open lots of tabs so that the tab bar overflows
2. select the first tab
3. press cmd+9 to select the last tab

Expected result:
The last tab scrolls into view. The down-scrolling button is disabled, as there's no other tab that can be scroll to in that direction.

Actual result:
The last tab scrolls into view. There's a bogus one-pixel-wide space next to the tab, because of which the down-scrolling button remains enabled. You can click the button to pass that space, after which the button will be disabled.

I'm not sure I see the gain that the margin adds visually. We should probably just remove it.

Comment 1

[Dão Gottwald [:dao]]

This is a regression from 3.0. The margin was introduced in bug 449832, and I reduced it to one pixel in bug 458111.

Comment 3

[Markus Stange [:mstange]]

(In reply to comment #0)
> I'm not sure I see the gain that the margin adds visually.

In bug 449832, the margin didn't result in more spacing between the tabs because it was compensated with negative margins on .tab-image-right and .tab-image-left. The whole setup was a workaround with the purpose of preserving a transparent tab shadow for the tab drag image. After bug 458111, which made the tabs image-less, and bug 225680, which replaced the tab drag image, this workaround became obsolete.

> We should probably just remove it.

Agreed.

Comment 4

[Dão Gottwald [:dao]]

(In reply to comment #3)
> (In reply to comment #0)
> > I'm not sure I see the gain that the margin adds visually.
> 
> In bug 449832, the margin didn't result in more spacing between the tabs
> because it was compensated with negative margins on .tab-image-right and
> .tab-image-left.

Yep, but bug 458111 didn't result in more spacing either. What I meant is that the space seems unnecessary, regardless of whether it's a margin or part of the element.

Btw, bug 449832 also affected the scrolling, although in a slightly different manner: the negative margin caused tabs to scroll only partly into view.

Comment 5

[Markus Stange [:mstange]]

(In reply to comment #4)
> Yep, but bug 458111 didn't result in more spacing either. What I meant is that
> the space seems unnecessary, regardless of whether it's a margin or part of the
> element.

Ah, got it. OK.

> Btw, bug 449832 also affected the scrolling, although in a slightly different
> manner: the negative margin caused tabs to scroll only partly into view.

Uh. Good that this hack is gone now.

Comment 6

[Dão Gottwald [:dao]]

(In reply to comment #5)
> (In reply to comment #4)
> > Yep, but bug 458111 didn't result in more spacing either. What I meant is that
> > the space seems unnecessary, regardless of whether it's a margin or part of the
> > element.
> 
> Ah, got it. OK.

So do you still agree that we should remove it, rather than looking for a different way to implement the gap?

Comment 7

[Markus Stange [:mstange]]

I agree. But you should ask Faaborg, too.

Maybe we should even implement his suggestion in bug 479869 comment 7 at the same time? (I don't like the current look of background tabs at all, to be honest.)

Comment 8

[Dão Gottwald [:dao]]

Attachment: remove it

Comment 9

[Dão Gottwald [:dao]]

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

Comment 10

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

Comment on attachment 367088 [details] [diff] [review]
remove it

a191=beltzner

Comment 11

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f5b14b151d2c

Comment 12

[Aaron Train [:aaronmt]]

Verified fixed in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090526 Shiretoko/3.5pre

Comment 13

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P2 - Polish issue that is in a secondary interface, occasionally encountered, and is easily identifiable.

impacted the main window, but probably not super noticeable and requires a full tab strip