625654 - Entering another tab doesn't trigger collapse of an expanded tab group

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P4)

Description

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Video

STR:
1. Get a stacked group.
2. Set up another tab elsewhere as the active tab.
3. Click on that first group's expander so it fans out ("expands").
4. Through the keyboard shortcut, for example, zoom out of Panorama. You will go into the active tab.

Expected:
This should trigger a collapsing of that expanded tab group.

Reality:
It does not.

This is a follow up bug from bug 625443. The browser_tabview_expander test written there has a todo item for this.

Comment 1

[Michael Yoshitaka Erlewine [:mitcho]]

Updated spec from Ian in bug 625443:

When we expand a stacked group, make one of those tabs (probably the top tab) active at that point. Thus, if we then activate Panorama via the key shortcut, it will go into a tab in that group. This avoids the availability of situation described above.

Comment 2

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Trivial patch (requires 625443 patch v1.1)

Ian's new spec for this behavior was ridculously easy to implement, and it now passes all of the TODO items for this bug in the expander test (from bug 625443), except for one: because we zoom into a tab and come back so quickly, the group may be marked as no longer expanded, but the overlay DOM element may still exist. Removed that test.

Comment 3

[Michael Yoshitaka Erlewine [:mitcho]]

Pushed to try, on top of bug 625443 patch v1.1.

Comment 4

[Michael Yoshitaka Erlewine [:mitcho]]

All but osx debug has passed, but my second osx debug push got eaten somehow by try, so pushed just osx debug again.

Comment 5

[Michael Yoshitaka Erlewine [:mitcho]]

Passed try!

Comment 6

[Ian Gilman [:iangilman]]

Comment on attachment 504170 [details] [diff] [review]
Trivial patch (requires 625443 patch v1.1)

Very nice

Comment 7

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

Comment on attachment 504170 [details] [diff] [review]
Trivial patch (requires 625443 patch v1.1)

a=beltzner, with thanks for the test!

Comment 8

[Michael Yoshitaka Erlewine [:mitcho]]

Attachment: Patch for checkin

Comment 9

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/13790fc912fa

Comment 10

[Tracy Walker [:tracy]]

verified with minefield build of 20110217