Closed Bug 482965 Opened 13 years ago Closed 13 years ago

tabbrowser-tabs shouldn't have horizontal margin

Categories

(Firefox :: Theme, defect)

3.5 Branch
x86
macOS
defect
Not set
trivial

Tracking

()

RESOLVED FIXED
Firefox 3.6a1

People

(Reporter: dao, Assigned: dao)

References

Details

(Keywords: regression, verified1.9.1, Whiteboard: [polish-easy] [polish-interactive][polish-p2])

Attachments

(1 file)

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.
This is a regression from 3.0. The margin was introduced in bug 449832, and I reduced it to one pixel in bug 458111.
Blocks: 449832
Keywords: regression
Version: Trunk → 3.1 Branch
Blocks: 458111
Duplicate of this bug: 477123
(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.
(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.
(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.
(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?
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.)
Attached patch remove itSplinter Review
Attachment #367088 - Flags: ui-review?(faaborg)
Whiteboard: [polish-easy] [polish-interactive]
Attachment #367088 - Flags: ui-review?(faaborg) → ui-review+
Attachment #367088 - Flags: review?(gavin.sharp)
Attachment #367088 - Flags: review?(gavin.sharp) → review+
http://hg.mozilla.org/mozilla-central/rev/dbdf1401b7e1
Assignee: nobody → dao
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.2a1
Attachment #367088 - Flags: approval1.9.1?
Comment on attachment 367088 [details] [diff] [review]
remove it

a191=beltzner
Attachment #367088 - Flags: approval1.9.1? → approval1.9.1+
Keywords: checkin-needed
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
Keywords: verified1.9.1
Keywords: fixed1.9.1
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
Whiteboard: [polish-easy] [polish-interactive] → [polish-easy] [polish-interactive][polish-p2]
You need to log in before you can comment on or make changes to this bug.