"Move to Group" should always insert the moved tab into the same place

VERIFIED FIXED in Firefox 9

Status

Firefox Graveyard
Panorama
VERIFIED FIXED
6 years ago
a year ago

People

(Reporter: ttaubert, Assigned: raymondlee)

Tracking

Trunk
Firefox 9

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
(Assignee)

Updated

6 years ago
Assignee: nobody → raymond
(Assignee)

Comment 1

6 years ago
At the moment, it moves tab to the first position of target group.  WIll make it add to the group as last element.
(Assignee)

Comment 2

6 years ago
Created attachment 561153 [details] [diff] [review]
v1
Attachment #561153 - Flags: review?(dietrich)
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?
Attachment #561153 - Flags: review?(dietrich) → review+
(Assignee)

Comment 4

6 years ago
(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.
(Assignee)

Comment 5

6 years ago
Created attachment 561372 [details] [diff] [review]
Patch for checkin
Attachment #561153 - Attachment is obsolete: true
(Assignee)

Comment 6

6 years ago
Comment on attachment 561372 [details] [diff] [review]
Patch for checkin

Passed Try!
https://tbpl.mozilla.org/?tree=Try&rev=73691b051cf8
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/a9edf67c8954
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/a9edf67c8954
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 9
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)
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.