Bug 1230831 - fix treatment of tabs with no group information, r=mconley

--HG--
extra : commitid : 5MCJ6ORdoaJ
extra : rebase_source : 3a554a60593b1164fed76fde9074cadbcfe230f3
This commit is contained in:
Gijs Kruitbosch 2015-12-14 13:49:13 +00:00
parent e1743f0fb6
commit d179d6529b
2 changed files with 161 additions and 14 deletions

View File

@ -76,6 +76,8 @@ this.TabGroupsMigrator = {
);
}
// We always write this back to ensure that any spurious tab groups data is
// removed:
stateAsSupportsString.data = JSON.stringify(state);
},
@ -99,15 +101,39 @@ this.TabGroupsMigrator = {
}
let windowGroupData = new Map();
let activeGroupID = null;
let tabsWithoutGroup = [];
for (let tab of win.tabs) {
let group;
// Get a string group ID:
try {
group = tab.extData && tab.extData["tabview-tab"] &&
(JSON.parse(tab.extData["tabview-tab"]).groupID + "");
let tabViewData = tab.extData && tab.extData["tabview-tab"] &&
JSON.parse(tab.extData["tabview-tab"]);
if (tabViewData && ("groupID" in tabViewData)) {
group = tabViewData.groupID + "";
}
} catch (ex) {
// Ignore tabs with no group info
continue;
// Ignore errors reading group info, treat as active group
}
if (!group) {
// We didn't find group info. If we already have an active group,
// pretend this is part of that group:
if (activeGroupID) {
group = activeGroupID;
} else {
if (!tabsWithoutGroup) {
Cu.reportError("ERROR: the list of tabs without groups was " +
"nulled out, but there's no active group ID? " +
"This should never happen!");
tabsWithoutGroup = [];
}
// Otherwise, add to the list of tabs with no group and move to
// the next tab immediately. We'll add all these tabs at the
// beginning of the active group as soon as we find a tab in it,
// so as to preserve their order.
tabsWithoutGroup.push(tab);
continue;
}
}
let groupData = windowGroupData.get(group);
if (!groupData) {
@ -119,11 +145,26 @@ this.TabGroupsMigrator = {
if (!title) {
groupData.anonGroupID = ++globalAnonGroupID;
}
// 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;
}
windowGroupData.set(group, groupData);
}
groupData.tabs.push(tab);
}
// If we found tabs but no active group, assume there's just 1 group:
if (tabsWithoutGroup && tabsWithoutGroup.length) {
windowGroupData.set("active group", {
tabs: tabsWithoutGroup,
anonGroupID: ++globalAnonGroupID,
});
}
allGroupData.set(win, windowGroupData);
}
}

View File

@ -80,6 +80,91 @@ const TEST_STATES = {
}
],
},
TAB_WITHOUT_GROUP: {
selectedWindow: 1,
windows: [
{
tabs: [
{
entries: [{
url: "about:robots",
title: "Robots 1",
}],
index: 1,
hidden: false,
},
{
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\":1}",
},
}
],
extData: {
"tabview-group": "{\"2\":{}, \"1\": {}}",
"tabview-groups": "{\"nextID\":20,\"activeGroupId\":2,\"totalNumber\":2}",
"tabview-visibility": "false"
},
}
],
},
ONLY_UNGROUPED_TABS: {
selectedWindow: 1,
windows: [
{
tabs: [
{
entries: [{
url: "about:robots",
title: "Robots 1",
}],
index: 1,
hidden: false,
},
{
entries: [{
url: "about:robots",
title: "Robots 2",
}],
index: 1,
hidden: false,
extData: {
"tabview-tab": "{}",
},
},
{
entries: [{
url: "about:robots",
title: "Robots 3",
}],
index: 1,
hidden: true,
extData: {
},
}
],
extData: {
"tabview-group": "{\"2\":{}}",
},
}
],
},
};
add_task(function* gatherGroupDataTest() {
@ -89,17 +174,14 @@ add_task(function* gatherGroupDataTest() {
Assert.equal(singleWinGroups.size, 2, "2 groups");
let group2 = singleWinGroups.get("2");
Assert.ok(!!group2, "group 2 should exist");
if (group2) {
Assert.equal(group2.tabs.length, 2, "2 tabs in group 2");
Assert.equal(group2.tabGroupsMigrationTitle, "", "We don't assign titles to untitled groups");
Assert.equal(group2.anonGroupID, "1", "We mark an untitled group with an anonymous id");
}
Assert.equal(group2.tabs.length, 2, "2 tabs in group 2");
Assert.equal(group2.tabGroupsMigrationTitle, "", "We don't assign titles to untitled groups");
Assert.equal(group2.anonGroupID, "1", "We mark an untitled group with an anonymous id");
let group13 = singleWinGroups.get("13");
if (group13) {
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");
}
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");
});
add_task(function* bookmarkingTest() {
@ -202,3 +284,27 @@ add_task(function* migrationPageDataTest() {
},
"Should have added expected tab at the end of the tab list.");
});
add_task(function* correctMissingTabGroupInfo() {
let stateClone = JSON.parse(JSON.stringify(TEST_STATES.TAB_WITHOUT_GROUP));
let groupInfo = TabGroupsMigrator._gatherGroupData(stateClone);
Assert.equal(groupInfo.size, 1, "Should have 1 window");
let windowGroups = [...groupInfo][0][1];
Assert.equal(windowGroups.size, 2, "Window should have 2 groups");
let group2 = windowGroups.get("2");
Assert.ok(group2, "Group 2 should exist");
Assert.equal(group2.tabs.length, 2, "There should be 2 tabs in group 2");
Assert.equal(group2.tabs[0].entries[0].title, "Robots 1", "The first tab of group 2 should be the tab with no group info.");
});
add_task(function* dealWithNoGroupInfo() {
let stateClone = JSON.parse(JSON.stringify(TEST_STATES.ONLY_UNGROUPED_TABS));
let groupInfo = TabGroupsMigrator._gatherGroupData(stateClone);
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");
});