606901 - Tab close button doesn't show after moving some tabs to other group

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P3)

Description

[ithinc]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101024 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101024 Firefox/4.0b8pre

After moving some tabs to another group, tab close buttons doesn't appear on the background tabs even when they're wider than tabClipWidth.

Reproducible: Always

Steps to Reproduce:
1. Open 10 tabs or so, and tab close buttons on background tabs disappear.
2. Move tabs to another group one by one.

Comment 1

[Raymond Lee [:raymondlee]]

Attachment: v1

Comment 2

[Ian Gilman [:iangilman]]

Comment on attachment 486668 [details] [diff] [review]
v1

Looks good to me.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 486668 [details] [diff] [review]
v1

It seems to me that this should be done as part of hideTab and showTab.

Comment 4

[Raymond Lee [:raymondlee]]

Attachment: v1

Based on dao's comment, put adjustTabstrip() in showTab() and hideTab()

Comment 5

[ithinc]

(In reply to comment #4)
> Based on dao's comment, put adjustTabstrip() in showTab() and hideTab()

Doesn't this add too many unnecessary adjustTabstrip calls? We don't have an UI command to trigger showTab/hideTab directly.

Comment 6

[ithinc]

I filed another bug 608184, which is also related to moveTabToGroup.

Comment 8

[Dão Gottwald [:dao]]

(In reply to comment #5)
> We don't have an UI command to trigger showTab/hideTab directly.

Doesn't matter -- It's part of the tabbrowser API.

Comment 9

[Raymond Lee [:raymondlee]]

Comment on attachment 486830 [details] [diff] [review]
v1

Passed try!

Comment 10

[Ian Gilman [:iangilman]]

Comment on attachment 486830 [details] [diff] [review]
v1

Looks good

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 486830 [details] [diff] [review]
v1

>+  // a setTimeout() in addTab is used to trigger adjustTabstrip() so we need a delay here as well.
>+  executeSoon(function() {
>+    is(gBrowser.tabContainer.getAttribute("closebuttons"), "activetab", "Only show button on selected tab.");

I wouldn't be surprised if this fails intermittently...

Comment 12

[Raymond Lee [:raymondlee]]

Could someone give approval2.0 to this please?

Comment 13

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

Comment on attachment 486830 [details] [diff] [review]
v1

a=beltzner

Comment 14

[Raymond Lee [:raymondlee]]

Attachment: Patch for check-in

Comment 15

[Raymond Lee [:raymondlee]]

Passed Try

Comment 16

[(no longer active)]

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

Comment 17

[Tracy Walker [:tracy]]

verified in current minefield nightly.