349751 - extra space between tabs after dragging a tab to extreme left

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

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

An interesting little bug, probably due to a lack of style overrides somewhere. Only seeing it on Mac. The result is that there's extra space between two tabs, which ends up looking pretty odd.

To reproduce:

1. Open a few tabs
2. Select the left-most tab
3. Drag a tab to the extreme left (so that it becomes the new left-most tab)
4. Take a look at the space between the two left-most tabs

For extra fun, drag another tab to the extreme left. You'll get the extra space between the three left-most tabs. Lather, rinse, repeat.

The extra space disappears when you select the tab to the right of the extra space.

Comment 1

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

Attachment: screenshot showing the extra space between leftmost tabs

Comment 2

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

--> sspitzer, per mconnor's decree!
--> blocking-firefox2? just so drivers see it, though I expect it to get +'d

Comment 3

[Mike Connor [:mconnor]]

so, this is a bug in tabbrowser, we don't update firsttab/lasttab/beforeselected/afterselected when moving tabs.  DOMI reveals the truth!  Might be as easy as duplicating the bits in the tab selection code.

Comment 4

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

Attachment: patch

Comment 5

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

the setter for "selected" on a tab (in tabbox.xml), does all the work of updating the "first-tab", "last-tab", "beforeselected" and "afterselected" attributes.

but it is per tab, so in moveTabTo() in tabbrowser.xml, we can't just do "this.mCurrentTab.selected = false;", change the position, and then do "this.mCurrentTab.selected = true;" to properly add (and remove) the "first-tab", "last-tab", "beforeselected" and "afterselected" attributes on all the tabs.

the fix I am proposing is similar to what we are doing in the setter for selectedIndex in tabbox.xml, which is, before doing tab.selected = true, we must first cal tab.selected = false on all the tabs.

thanks to mconnor for his reconnaissance work with DOMi and his suggestion.

Comment 6

[Mike Connor [:mconnor]]

Comment on attachment 235254 [details] [diff] [review]
patch

r=me on this, we should have always done this!

Comment 7

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

Comment on attachment 235254 [details] [diff] [review]
patch

seeking approval

Comment 8

[Mike Schroepfer]

Does this bug affect trunk?  I can't seem to tell - but the tabs are ugly there anyway :-).

Comment 9

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

> Does this bug affect trunk?  I can't seem to tell - but the tabs are ugly there
> anyway :-).

it does affect the trunk, but not as noticeably.  when you hit this bug on the trunk you would see that the second tab has shifted 3px.

I'll attach as screen shot of what I mean.

Comment 10

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

fix landed on the trunk, seeking approval for the branch.

Comment 11

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

Attachment: screen shot of this bug shows up on the mac (notice how the the right most close box and blogger favicon are not aligned between the top and bottom windows)

once you are in this state, try clicking the 1st and 2nd tabs, and you will see the alignment problem as clicking tabs will clean things up.

Comment 12

[Wayne Woods]

Out of interest, see bug 342679 comment 6. The margin-left set for the first-tab is revealing this problem, even though it isn't the underlying bug here. However, I can't see any reason for setting that margin. It seems to do nothing more than distort/shrink the first tab.

Comment 13

[Mike Schroepfer]

Comment on attachment 235254 [details] [diff] [review]
patch

a=schrep/beltnzer for drivers.

Comment 14

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

fix landed on the branch.

(wayne, I'll respond to your comment next.)