Last Comment Bug 685692 - "Move to Group" should always insert the moved tab into the same place
: "Move to Group" should always insert the moved tab into the same place
Status: VERIFIED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: -- normal
: Firefox 9
Assigned To: Raymond Lee [:raymondlee]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-08 14:24 PDT by Tim Taubert [:ttaubert]
Modified: 2016-04-12 14:00 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (4.05 KB, patch)
2011-09-20 02:59 PDT, Raymond Lee [:raymondlee]
dietrich: review+
Details | Diff | Review
Patch for checkin (4.33 KB, patch)
2011-09-20 19:52 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Review

Description Tim Taubert [:ttaubert] 2011-09-08 14:24:10 PDT
When using the tab context menu item "Move to Group" we should insert the moved tab always into the same place. I'd say we append it but the UX team might oppose that. Asking the UX team where to insert those tabs.
Comment 1 Raymond Lee [:raymondlee] 2011-09-19 22:02:48 PDT
At the moment, it moves tab to the first position of target group.  WIll make it add to the group as last element.
Comment 2 Raymond Lee [:raymondlee] 2011-09-20 02:59:40 PDT
Created attachment 561153 [details] [diff] [review]
v1
Comment 3 Dietrich Ayala (:dietrich) 2011-09-20 13:49:04 PDT
Comment on attachment 561153 [details] [diff] [review]
v1

Review of attachment 561153 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/tabview/groupitems.js
@@ +2604,5 @@
>      // add tab item to a groupItem
>      if (groupItemId) {
>        groupItem = GroupItems.groupItem(groupItemId);
>        groupItem.add(tab._tabViewTabItem);
> +      groupItem.reorderTabsBasedOnTabItemOrder()

looks fine, r=me. and btw, i totally dig how readable panorama tests are.

one question though: should these two methods result in a different order? what's the use-case for a reordering that puts an appended tab at the front of a group?
Comment 4 Raymond Lee [:raymondlee] 2011-09-20 19:29:55 PDT
(In reply to Dietrich Ayala (:dietrich) from comment #3)
> one question though: should these two methods result in a different order?
> -      UI.setReorderTabItemsOnShow(groupItem);
This method is for reordering tab items in Panorama based on the tab order on the tab bar.  The issue of this bug is that the tabs on tab bar is not reordered before entering Panorama, therefore, the moved tab appears in the new group in Panorama in the order of tabs on tabbar.

> +      groupItem.reorderTabsBasedOnTabItemOrder()
To solve this, we use the above method to keep the order of tabs on tab bar and tab items in Panorama in sync.  When a tab is moved to another group  using "Move to Group" menu item, the moved tab item in Panorama would be appended to the new group list. we call the above method so when user enters next time, the moved tab would appear at the end of list.


> what's the use-case for a reordering that puts an appended tab at the front
> of a group?
For example, 

In Panorama
Tab 1, Tab 2 in Group A
Tab 3, Tab 4 in Group B

Group A is being displayed on tab bar.  Here is the tab order on tab bar
Tab 1, Tab 2, Tab 3 (hidden), Tab 4 (hidden)

Move Tab 1 to Group B and then enter Panorama. The items in Group B is reordered based on tab order on tab bar.  Tab 1 appears as the first item in Group B 
Tab 1 in Group A
Tab 1, Tab 3, Tab 4 in Group B

The patch solves this problem.
Comment 5 Raymond Lee [:raymondlee] 2011-09-20 19:52:58 PDT
Created attachment 561372 [details] [diff] [review]
Patch for checkin
Comment 6 Raymond Lee [:raymondlee] 2011-09-20 20:03:36 PDT
Comment on attachment 561372 [details] [diff] [review]
Patch for checkin

Passed Try!
https://tbpl.mozilla.org/?tree=Try&rev=73691b051cf8
Comment 7 Tim Taubert [:ttaubert] 2011-09-26 19:47:19 PDT
https://hg.mozilla.org/integration/fx-team/rev/a9edf67c8954
Comment 8 Tim Taubert [:ttaubert] 2011-09-27 04:37:17 PDT
https://hg.mozilla.org/mozilla-central/rev/a9edf67c8954
Comment 9 Virgil Dicu [:virgil] [QA] 2011-10-26 08:44:24 PDT
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20111026 Firefox/9.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111026 Firefox/10.0a1

When using move to group from the tab context menu, the moved tab is now always inserted in the last position. (as specified in comment 2). Verified on Windows XP, 7, Ubuntu 11.10, Mac OS 10.6 on Firefox 9 and 10 (Aurora and Nightly)

Note You need to log in before you can comment on or make changes to this bug.