Bug 1355663 - Don't de-duplicate tabs from multiple devices. r?eoger
MozReview-Commit-ID: AciCTECtXJc
--- a/services/sync/modules/SyncedTabs.jsm
+++ b/services/sync/modules/SyncedTabs.jsm
@@ -105,47 +105,36 @@ let SyncedTabsInternal = {
return result;
}
// A boolean that controls whether we should show the icon from the remote tab.
const showRemoteIcons = Preferences.get("services.sync.syncedTabs.showRemoteIcons", true);
let engine = Weave.Service.engineManager.get("tabs");
- let seenURLs = new Set();
let ntabs = 0;
for (let client of Object.values(engine.getAllClients())) {
if (!Weave.Service.clientsEngine.remoteClientExists(client.id)) {
continue;
}
let clientRepr = await this._makeClient(client);
log.debug("Processing client", clientRepr);
for (let tab of client.tabs) {
let url = tab.urlHistory[0];
log.debug("remote tab", url);
- // Note there are some issues with tracking "seen" tabs, including:
- // * We really can't return the entire urlHistory record as we are
- // only checking the first entry - others might be different.
- // * We don't update the |lastUsed| timestamp to reflect the
- // most-recently-seen time.
- // In a followup we should consider simply dropping this |seenUrls|
- // check and return duplicate records - it seems the user will be more
- // confused by tabs not showing up on a device (because it was detected
- // as a dupe so it only appears on a different device) than being
- // confused by seeing the same tab on different clients.
- if (!url || seenURLs.has(url)) {
+
+ if (!url) {
continue;
}
let tabRepr = await this._makeTab(client, tab, url, showRemoteIcons);
if (filter && !this._tabMatchesFilter(tabRepr, filter)) {
continue;
}
- seenURLs.add(url);
clientRepr.tabs.push(tabRepr);
}
// We return all clients, even those without tabs - the consumer should
// filter it if they care.
ntabs += clientRepr.tabs.length;
result.push(clientRepr);
}
log.info(`Final tab list has ${result.length} clients with ${ntabs} tabs.`);
--- a/services/sync/tests/unit/test_syncedtabs.js
+++ b/services/sync/tests/unit/test_syncedtabs.js
@@ -212,8 +212,37 @@ add_task(async function test_filter() {
equal(clients[0].tabs.length, 1);
equal(clients[0].tabs[0].url, "http://foo.com/");
// check it matches the title.
clients = await SyncedTabs.getTabClients("test");
equal(clients.length, 1);
equal(clients[0].tabs.length, 1);
equal(clients[0].tabs[0].url, "http://foo.com/");
});
+
+add_task(async function test_duplicatesTabsAcrossClients() {
+
+ await configureClients({
+ guid_desktop: {
+ clientName: "My Desktop",
+ tabs: [
+ {
+ urlHistory: ["http://foo.com/"],
+ title: "A test page.",
+ }],
+ },
+ guid_mobile: {
+ clientName: "My Phone",
+ tabs: [
+ {
+ urlHistory: ["http://foo.com/"],
+ title: "A test page.",
+ }],
+ },
+ });
+
+ let clients = await SyncedTabs.getTabClients();
+ equal(clients.length, 2);
+ equal(clients[0].tabs.length, 1);
+ equal(clients[1].tabs.length, 1);
+ equal(clients[0].tabs[0].url, "http://foo.com/");
+ equal(clients[1].tabs[0].url, "http://foo.com/");
+});