--- a/browser/modules/TabGroupsMigrator.jsm
+++ b/browser/modules/TabGroupsMigrator.jsm
@@ -139,16 +139,19 @@ this.TabGroupsMigrator = {
if (!groupData) {
let title = (groupInfo[group] && groupInfo[group].title) || "";
groupData = {
tabs: [],
tabGroupsMigrationTitle: title,
};
if (!title) {
groupData.anonGroupID = ++globalAnonGroupID;
+ groupData.tabGroupsMigrationTitle =
+ gBrowserBundle.formatStringFromName("tabgroups.migration.anonGroup",
+ [groupData.anonGroupID], 1);
}
// If this is the active group, set the active group ID and add
// all the already-known tabs (that didn't list a group ID), if any.
if (!activeGroupID && !tab.hidden) {
activeGroupID = group;
groupData.tabs = tabsWithoutGroup;
tabsWithoutGroup = null;
}
@@ -174,42 +177,43 @@ this.TabGroupsMigrator = {
_createBackup(stateStr) {
let dest = Services.dirsvc.get("ProfD", Ci.nsIFile);
dest.append("tabgroups-session-backup.json");
let promise = OS.File.writeAtomic(dest.path, stateStr, {encoding: "utf-8"});
AsyncShutdown.webWorkersShutdown.addBlocker("TabGroupsMigrator", promise);
return promise;
},
+ _groupSorter(a, b) {
+ if (!a.anonGroupID) {
+ return -1;
+ }
+ if (!b.anonGroupID) {
+ return 1;
+ }
+ return a.anonGroupID - b.anonGroupID;
+ },
+
_bookmarkAllGroupsFromState: Task.async(function*(groupData) {
// First create a folder in which to put all these bookmarks:
this.bookmarkedGroupsPromise = PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.menuGuid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
index: 0,
title: gBrowserBundle.GetStringFromName("tabgroups.migration.tabGroupBookmarkFolderName"),
}).catch(Cu.reportError);
let tabgroupsFolder = yield this.bookmarkedGroupsPromise;
for (let [, windowGroupMap] of groupData) {
- let windowGroups = [... windowGroupMap.values()].sort((a, b) => {
- if (!a.anonGroupID) {
- return -1;
- }
- if (!b.anonGroupID) {
- return 1;
- }
- return a.anonGroupID - b.anonGroupID;
- });
+ let windowGroups = [... windowGroupMap.values()].sort(this._groupSorter);
for (let group of windowGroups) {
let groupFolder = yield PlacesUtils.bookmarks.insert({
parentGuid: tabgroupsFolder.guid,
type: PlacesUtils.bookmarks.TYPE_FOLDER,
- title: group.tabGroupsMigrationTitle ||
- gBrowserBundle.formatStringFromName("tabgroups.migration.anonGroup", [group.anonGroupID], 1),
+ title: group.tabGroupsMigrationTitle
}).catch(Cu.reportError);
for (let tab of group.tabs) {
let entry = tab.entries[tab.index - 1];
yield PlacesUtils.bookmarks.insert({
parentGuid: groupFolder.guid,
title: tab.title || entry.title,
url: entry.url,
@@ -248,22 +252,25 @@ this.TabGroupsMigrator = {
// Make sure we unhide it, or it won't get restored correctly
tab.hidden = false;
}
}
// We then convert any hidden groups into windows for the state object
// we show in about:tabgroupsdata
if (groupInfoForWindow) {
+ let windowsToReturn = [];
for (let groupID of hiddenGroupIDs) {
let group = groupInfoForWindow.get("" + groupID);
if (group) {
- stateToReturn.windows.push(group);
+ windowsToReturn.push(group);
}
}
+ windowsToReturn.sort(this._groupSorter);
+ stateToReturn.windows = stateToReturn.windows.concat(windowsToReturn);
}
// Finally we remove tab groups data from the window:
if (win.extData) {
delete win.extData["tabview-group"];
delete win.extData["tabview-groups"];
delete win.extData["tabview-ui"];
delete win.extData["tabview-visibility"];
--- a/browser/modules/test/xpcshell/test_TabGroupsMigrator.js
+++ b/browser/modules/test/xpcshell/test_TabGroupsMigrator.js
@@ -160,27 +160,109 @@ const TEST_STATES = {
}
],
extData: {
"tabview-group": "{\"2\":{}}",
},
}
],
},
+ SORTING_NAMING_RESTORE_PAGE: {
+ windows: [
+ {
+ tabs: [
+ {
+ entries: [{
+ url: "about:robots",
+ title: "Robots 1",
+ }],
+ index: 1,
+ hidden: false,
+ extData: {
+ "tabview-tab": "{\"groupID\":2,\"active\":true}",
+ },
+ },
+ {
+ entries: [{
+ url: "about:robots",
+ title: "Robots 2",
+ }],
+ index: 1,
+ hidden: false,
+ extData: {
+ "tabview-tab": "{\"groupID\":2}",
+ },
+ },
+ {
+ entries: [{
+ url: "about:robots",
+ title: "Robots 3",
+ }],
+ index: 1,
+ hidden: true,
+ extData: {
+ "tabview-tab": "{\"groupID\":13}",
+ },
+ },
+ {
+ entries: [{
+ url: "about:robots",
+ title: "Robots 4",
+ }],
+ index: 1,
+ hidden: true,
+ extData: {
+ "tabview-tab": "{\"groupID\":15}",
+ },
+ },
+ {
+ entries: [{
+ url: "about:robots",
+ title: "Robots 5",
+ }],
+ index: 1,
+ hidden: true,
+ extData: {
+ "tabview-tab": "{\"groupID\":16}",
+ },
+ },
+ {
+ entries: [{
+ url: "about:robots",
+ title: "Robots 6",
+ }],
+ index: 1,
+ hidden: true,
+ extData: {
+ "tabview-tab": "{\"groupID\":17}",
+ },
+ }
+ ],
+ extData: {
+ "tabview-group": "{\"2\":{},\"13\":{\"title\":\"Foopy\"}, \"15\":{\"title\":\"Barry\"}, \"16\":{}, \"17\":{}}",
+ "tabview-groups": "{\"nextID\":20,\"activeGroupId\":2,\"totalNumber\":5}",
+ "tabview-visibility": "false"
+ },
+ },
+ ]
+ },
};
add_task(function* gatherGroupDataTest() {
let groupInfo = TabGroupsMigrator._gatherGroupData(TEST_STATES.TWO_GROUPS);
Assert.equal(groupInfo.size, 1, "Information about 1 window");
let singleWinGroups = [... groupInfo.values()][0];
Assert.equal(singleWinGroups.size, 2, "2 groups");
let group2 = singleWinGroups.get("2");
Assert.ok(!!group2, "group 2 should exist");
Assert.equal(group2.tabs.length, 2, "2 tabs in group 2");
- Assert.equal(group2.tabGroupsMigrationTitle, "", "We don't assign titles to untitled groups");
+ // Note that this has groupID 2 in the internal representation of tab groups,
+ // but because it was the first group we encountered when migrating, it was
+ // labeled "group 1" for the user
+ Assert.equal(group2.tabGroupsMigrationTitle, "Group 1", "We assign a numeric title to untitled groups");
Assert.equal(group2.anonGroupID, "1", "We mark an untitled group with an anonymous id");
let group13 = singleWinGroups.get("13");
Assert.ok(!!group13, "group 13 should exist");
Assert.equal(group13.tabs.length, 1, "1 tabs in group 13");
Assert.equal(group13.tabGroupsMigrationTitle, "Foopy", "Group with title has correct title");
Assert.ok(!("anonGroupID" in group13), "We don't mark a titled group with an anonymous id");
});
@@ -303,8 +385,22 @@ add_task(function* dealWithNoGroupInfo()
Assert.equal(groupInfo.size, 1, "Should have 1 window");
let windowGroups = [...groupInfo][0][1];
Assert.equal(windowGroups.size, 1, "Window should have 1 group");
let fallbackActiveGroup = windowGroups.get("active group");
Assert.ok(fallbackActiveGroup, "Fallback group should exist");
Assert.equal(fallbackActiveGroup.tabs.length, 3, "There should be 3 tabs in the fallback group");
});
+add_task(function* groupSortingInRemovedDataUsedForRestorePage() {
+ let stateClone = JSON.parse(JSON.stringify(TEST_STATES.SORTING_NAMING_RESTORE_PAGE));
+ let groupInfo = TabGroupsMigrator._gatherGroupData(stateClone);
+ let removedGroups = TabGroupsMigrator._removeHiddenTabGroupsFromState(stateClone, groupInfo);
+ Assert.equal(stateClone.windows.length, 1, "Should still only have 1 window");
+ Assert.equal(stateClone.windows[0].tabs.length, 2, "Should now have 2 tabs");
+
+ let restoredWindowTitles = removedGroups.windows.map(win => win.tabGroupsMigrationTitle);
+ // Note that group 1 is the active group and as such it won't appear in the list of
+ // things the user can restore:
+ Assert.deepEqual(restoredWindowTitles,
+ ["Barry", "Foopy", "Group 2", "Group 3"]);
+});
+