Skip to content

Commit bc20fc6

Browse files
committed
Fixed issue #16: Do not duplicate tabs on a closed space when reopening via Spaces window tab-clicking.
1 parent b351d73 commit bc20fc6

File tree

6 files changed

+179
-11
lines changed

6 files changed

+179
-11
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ All notable changes to this project will be documented in this file.
66

77
### Changes
88

9+
- Fixed [issue #16](https://github.com/codedread/spaces/issues/16): Stop duplicating tabs of a closed Space when tab is clicked from the Spaces window.
910
- Fixed [issue #14](https://github.com/codedread/spaces/issues/14): Open Spaces windows on the currently-active display.
10-
- Increased unit test coverage from 8.11% to 8.72%.
11+
- Increased unit test coverage from 8.11% to 9.32%.
1112

1213

1314
## [1.1.5] - 2025-09-08

js/background/background.js

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ async function rediscoverWindowByUrl(storageKey, htmlFilename) {
5555
}
5656

5757
export function initializeServiceWorker() {
58-
console.log(`Initializing service worker...(1.1.5)`);
58+
console.log(`Initializing service worker...`);
5959

6060
chrome.runtime.onInstalled.addListener(details => {
6161
console.log(`Extension installed: ${JSON.stringify(details)}`);
@@ -883,10 +883,7 @@ async function handleLoadSession(sessionId, tabUrl) {
883883
if (curSessionTab.pinned) {
884884
let pinnedTabId = false;
885885
newWindow.tabs.some(curNewTab => {
886-
if (
887-
curNewTab.url === curSessionTab.url ||
888-
curNewTab.pendingUrl === curSessionTab.url
889-
) {
886+
if (getEffectiveTabUrl(curNewTab) === curSessionTab.url) {
890887
pinnedTabId = curNewTab.id;
891888
return true;
892889
}
@@ -933,16 +930,16 @@ async function focusWindow(windowId) {
933930

934931
async function focusOrLoadTabInWindow(window, tabUrl) {
935932
let match = false;
936-
for (const tab of window.tabs) {
937-
if (tab.url === tabUrl) {
933+
for (const tab of window.tabs || []) {
934+
if (getEffectiveTabUrl(tab) === tabUrl) {
938935
await chrome.tabs.update(tab.id, { active: true });
939936
match = true;
940937
break;
941938
}
942939
}
943940

944941
if (!match) {
945-
await chrome.tabs.create({ url: tabUrl });
942+
await chrome.tabs.create({ url: tabUrl, active: true });
946943
}
947944
}
948945

@@ -1254,6 +1251,20 @@ function moveTabToWindow(tab, windowId) {
12541251
spacesService.queueWindowEvent(windowId);
12551252
}
12561253

1254+
// Module-level helper functions.
1255+
1256+
/**
1257+
* Gets the effective URL of a tab, preferring pendingUrl for loading tabs.
1258+
* @param {chrome.tabs.Tab} tab The tab object to get the effective URL from.
1259+
* @returns {string} The effective URL of the tab.
1260+
*/
1261+
function getEffectiveTabUrl(tab) {
1262+
if (tab.status === 'loading' && tab.pendingUrl) {
1263+
return tab.pendingUrl;
1264+
}
1265+
return tab.url;
1266+
}
1267+
12571268
/**
12581269
* Determines the most appropriate display to show a new window on.
12591270
* It prefers the display containing the currently focused Chrome window.
@@ -1286,4 +1297,9 @@ async function getTargetDisplayWorkArea() {
12861297
}
12871298

12881299
// Exports for testing.
1289-
export { cleanParameter, getTargetDisplayWorkArea };
1300+
export {
1301+
cleanParameter,
1302+
focusOrLoadTabInWindow,
1303+
getEffectiveTabUrl,
1304+
getTargetDisplayWorkArea,
1305+
};

manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "Spaces",
33
"description": "Intuitive tab management",
4-
"version": "1.1.6.0",
4+
"version": "1.1.6.1",
55
"permissions": [
66
"contextMenus",
77
"favicon",
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
2+
import { setupChromeMocks, jest } from './helpers.js';
3+
4+
jest.mock('../js/background/spacesService.js', () => ({
5+
cleanUrl: jest.fn(url => url),
6+
}));
7+
8+
describe('focusOrLoadTabInWindow', () => {
9+
let focusOrLoadTabInWindow;
10+
11+
beforeEach(async () => {
12+
setupChromeMocks();
13+
const background = await import('../js/background/background.js');
14+
focusOrLoadTabInWindow = background.focusOrLoadTabInWindow;
15+
});
16+
17+
afterEach(() => {
18+
jest.clearAllMocks();
19+
});
20+
21+
test('should focus existing tab when match is found', async () => {
22+
const window = {
23+
tabs: [
24+
{ id: 1, url: 'https://example.com', status: 'complete' },
25+
{ id: 2, url: 'https://google.com', status: 'complete' },
26+
],
27+
};
28+
const tabUrl = 'https://example.com';
29+
30+
await focusOrLoadTabInWindow(window, tabUrl);
31+
32+
expect(chrome.tabs.update).toHaveBeenCalledWith(1, { active: true });
33+
expect(chrome.tabs.create).not.toHaveBeenCalled();
34+
});
35+
36+
test('should create new tab when no match is found', async () => {
37+
const window = {
38+
tabs: [
39+
{ id: 1, url: 'https://google.com', status: 'complete' },
40+
{ id: 2, url: 'https://github.com', status: 'complete' },
41+
],
42+
};
43+
const tabUrl = 'https://example.com';
44+
45+
await focusOrLoadTabInWindow(window, tabUrl);
46+
47+
expect(chrome.tabs.update).not.toHaveBeenCalled();
48+
expect(chrome.tabs.create).toHaveBeenCalledWith({ url: tabUrl, active: true });
49+
});
50+
51+
test('should handle window with no tabs gracefully', async () => {
52+
const window = { tabs: [] };
53+
const tabUrl = 'https://example.com';
54+
55+
await focusOrLoadTabInWindow(window, tabUrl);
56+
57+
expect(chrome.tabs.update).not.toHaveBeenCalled();
58+
expect(chrome.tabs.create).toHaveBeenCalledWith({ url: tabUrl, active: true });
59+
});
60+
61+
test('should handle window with missing tabs property', async () => {
62+
const window = {};
63+
const tabUrl = 'https://example.com';
64+
65+
await focusOrLoadTabInWindow(window, tabUrl);
66+
67+
expect(chrome.tabs.update).not.toHaveBeenCalled();
68+
expect(chrome.tabs.create).toHaveBeenCalledWith({ url: tabUrl, active: true });
69+
});
70+
});

tests/getEffectiveTabUrl.test.js

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import { getEffectiveTabUrl } from '../js/background/background.js';
2+
3+
describe('getEffectiveTabUrl', () => {
4+
test('should return tab URL when status is complete', () => {
5+
const tab = {
6+
id: 1,
7+
url: 'https://example.com?param=value',
8+
status: 'complete'
9+
};
10+
11+
const result = getEffectiveTabUrl(tab);
12+
13+
// Should return URL as-is (no cleaning)
14+
expect(result).toBe('https://example.com?param=value');
15+
});
16+
17+
test('should return pendingUrl when status is loading and pendingUrl exists', () => {
18+
const tab = {
19+
id: 1,
20+
url: 'about:blank',
21+
pendingUrl: 'https://example.com/path#section',
22+
status: 'loading'
23+
};
24+
25+
const result = getEffectiveTabUrl(tab);
26+
27+
// Should use pendingUrl as-is (no cleaning)
28+
expect(result).toBe('https://example.com/path#section');
29+
});
30+
31+
test('should return tab URL when status is loading but no pendingUrl', () => {
32+
const tab = {
33+
id: 1,
34+
url: 'https://example.com?foo=bar',
35+
status: 'loading'
36+
};
37+
38+
const result = getEffectiveTabUrl(tab);
39+
40+
expect(result).toBe('https://example.com?foo=bar');
41+
});
42+
43+
test('should return tab URL when pendingUrl exists but status is not loading', () => {
44+
const tab = {
45+
id: 1,
46+
url: 'https://example.com?foo=bar',
47+
pendingUrl: 'https://different.com',
48+
status: 'complete'
49+
};
50+
51+
const result = getEffectiveTabUrl(tab);
52+
53+
expect(result).toBe('https://example.com?foo=bar');
54+
});
55+
56+
test('should handle tab with no status property', () => {
57+
const tab = {
58+
id: 1,
59+
url: 'https://example.com#hash'
60+
};
61+
62+
const result = getEffectiveTabUrl(tab);
63+
64+
expect(result).toBe('https://example.com#hash');
65+
});
66+
67+
test('should handle tab with empty pendingUrl', () => {
68+
const tab = {
69+
id: 1,
70+
url: 'https://example.com?query=test',
71+
pendingUrl: '',
72+
status: 'loading'
73+
};
74+
75+
const result = getEffectiveTabUrl(tab);
76+
77+
expect(result).toBe('https://example.com?query=test');
78+
});
79+
});

tests/helpers.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ export const setupChromeMocks = () => {
2121
},
2222
},
2323
tabs: {
24+
create: jest.fn(),
2425
query: jest.fn(),
26+
update: jest.fn(),
2527
},
2628
windows: {
2729
getCurrent: jest.fn(),

0 commit comments

Comments
 (0)