Skip to content

Commit ece5859

Browse files
committed
Fix issue #10 - ensure the stored window bounds fits in the currently active display (otherwise use the fallback dimensions)
1 parent 253e25b commit ece5859

File tree

5 files changed

+154
-27
lines changed

5 files changed

+154
-27
lines changed

CHANGELOG.md

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

77
### Changes
88

9-
- Work on [issue #10](https://github.com/codedread/spaces/issues/10): Remember named space window bounds in the internal database.
9+
- Fixed [issue #10](https://github.com/codedread/spaces/issues/10): Display space window bounds at their last known position and size.
1010
- Fixed [issue #20](https://github.com/codedread/spaces/issues/20): Close browser windows from the Spaces window.
11-
- Increased unit test coverage from 10.75% to 16.58%.
11+
- Increased unit test coverage from 10.75% to 16.35%.
1212

1313
## [1.1.6] - 2025-09-17
1414

js/background/background.js

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -840,24 +840,16 @@ async function handleLoadSession(sessionId, tabUrl) {
840840
return curTab.url;
841841
});
842842

843-
// Display new session with restored bounds if available, otherwise use default display area.
844-
let windowOptions = { url: urls };
845-
846-
if (session.windowBounds) {
847-
// Use stored window bounds for restoration
848-
windowOptions.height = session.windowBounds.height;
849-
windowOptions.width = session.windowBounds.width;
850-
windowOptions.top = session.windowBounds.top;
851-
windowOptions.left = session.windowBounds.left;
852-
} else {
853-
// Fallback to default display area positioning
854-
const workArea = await getTargetDisplayWorkArea();
855-
windowOptions.height = workArea.height - 100;
856-
windowOptions.width = workArea.width - 100;
857-
windowOptions.top = workArea.top;
858-
windowOptions.left = workArea.left;
859-
}
860-
843+
// Display new session with calculated bounds
844+
const workArea = await getTargetDisplayWorkArea();
845+
const bounds = calculateSessionBounds(workArea, session.windowBounds);
846+
let windowOptions = {
847+
url: urls,
848+
height: bounds.height,
849+
width: bounds.width,
850+
top: bounds.top,
851+
left: bounds.left
852+
};
861853
const newWindow = await chrome.windows.create(windowOptions);
862854

863855
// force match this new window to the session
@@ -1225,6 +1217,40 @@ function moveTabToWindow(tab, windowId) {
12251217

12261218
// Module-level helper functions.
12271219

1220+
/**
1221+
* Determines the window bounds to use for a session restore.
1222+
* @param {WindowBounds} displayBounds - The target display work area bounds
1223+
* @param {WindowBounds} sessionBounds - The stored session bounds
1224+
* @returns {WindowBounds} - The bounds to use for the window
1225+
*/
1226+
function calculateSessionBounds(displayBounds, sessionBounds) {
1227+
if (!sessionBounds
1228+
|| typeof sessionBounds.left !== 'number'
1229+
|| typeof sessionBounds.top !== 'number'
1230+
|| typeof sessionBounds.width !== 'number'
1231+
|| typeof sessionBounds.height !== 'number'
1232+
|| sessionBounds.left < displayBounds.left
1233+
|| sessionBounds.top < displayBounds.top
1234+
|| sessionBounds.left + sessionBounds.width > displayBounds.left + displayBounds.width
1235+
|| sessionBounds.top + sessionBounds.height > displayBounds.top + displayBounds.height
1236+
) {
1237+
// Fallback to display default area positioning (minus offset)
1238+
return {
1239+
left: displayBounds.left,
1240+
top: displayBounds.top,
1241+
width: displayBounds.width - 100,
1242+
height: displayBounds.height - 100,
1243+
};
1244+
}
1245+
// Otherwise, use the stored session bounds
1246+
return {
1247+
left: sessionBounds.left,
1248+
top: sessionBounds.top,
1249+
width: sessionBounds.width,
1250+
height: sessionBounds.height
1251+
};
1252+
}
1253+
12281254
/**
12291255
* Ensures the parameter is a number.
12301256
* @param {string|number} param - The parameter to clean.
@@ -1352,6 +1378,7 @@ async function requestAllSpaces() {
13521378

13531379
// Exports for testing.
13541380
export {
1381+
calculateSessionBounds,
13551382
cleanParameter,
13561383
focusOrLoadTabInWindow,
13571384
getEffectiveTabUrl,

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.7.2",
4+
"version": "1.1.7.3",
55
"permissions": [
66
"contextMenus",
77
"favicon",
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { calculateSessionBounds } from '../js/background/background.js';
2+
3+
const DISPLAY = { left: 0, top: 0, width: 1920, height: 1080 };
4+
const OFFSET = 100;
5+
const FALLBACK_BOUNDS = {
6+
left: 0,
7+
top: 0,
8+
width: 1920 - OFFSET,
9+
height: 1080 - OFFSET
10+
};
11+
12+
describe('calculateSessionBounds', () => {
13+
it('returns session bounds if fully within display', () => {
14+
const session = { left: 100, top: 100, width: 800, height: 600 };
15+
expect(calculateSessionBounds(DISPLAY, session)).toEqual(session);
16+
});
17+
18+
it('falls back to display area if session bounds are missing', () => {
19+
expect(calculateSessionBounds(DISPLAY, undefined)).toEqual(FALLBACK_BOUNDS);
20+
});
21+
22+
it('falls back if session bounds are partially outside display', () => {
23+
const session = { left: 1800, top: 900, width: 300, height: 300 };
24+
expect(calculateSessionBounds(DISPLAY, session)).toEqual(FALLBACK_BOUNDS);
25+
});
26+
27+
it('falls back if session bounds are negative', () => {
28+
const session = { left: -100, top: -100, width: 400, height: 300 };
29+
expect(calculateSessionBounds(DISPLAY, session)).toEqual(FALLBACK_BOUNDS);
30+
});
31+
32+
it('falls back if session bounds are too large', () => {
33+
const session = { left: 0, top: 0, width: 3000, height: 2000 };
34+
expect(calculateSessionBounds(DISPLAY, session)).toEqual(FALLBACK_BOUNDS);
35+
});
36+
37+
it('falls back if session bounds properties are not numbers', () => {
38+
const session = { left: '100', top: null, width: undefined, height: NaN };
39+
expect(calculateSessionBounds(DISPLAY, session)).toEqual(FALLBACK_BOUNDS);
40+
});
41+
42+
it('falls back if session bounds object is missing properties', () => {
43+
const session = { left: 100, width: 800 };
44+
expect(calculateSessionBounds(DISPLAY, session)).toEqual(FALLBACK_BOUNDS);
45+
});
46+
47+
it('falls back if session bounds is null', () => {
48+
expect(calculateSessionBounds(DISPLAY, null)).toEqual(FALLBACK_BOUNDS);
49+
});
50+
});

tests/handleLoadSession.test.js

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ describe('handleLoadSession', () => {
206206
});
207207
});
208208

209-
test('should handle partially defined bounds (gracefully fail to fallback)', async () => {
209+
test('should handle partially defined bounds (falls back to display area)', async () => {
210210
mockSession.windowBounds = {
211211
left: 100,
212212
top: 50
@@ -215,13 +215,63 @@ describe('handleLoadSession', () => {
215215

216216
await handleLoadSession(mockSession.id);
217217

218-
// Should use stored bounds even if incomplete
218+
// Should fallback to display area bounds
219219
expect(global.chrome.windows.create).toHaveBeenCalledWith({
220220
url: ['https://example.com', 'https://test.com'],
221-
height: undefined,
222-
width: undefined,
223-
top: 50,
224-
left: 100
221+
height: 980,
222+
width: 1820,
223+
top: 0,
224+
left: 0
225+
});
226+
});
227+
});
228+
229+
describe('bounds edge cases with display area', () => {
230+
test('should fallback if session bounds are negative', async () => {
231+
mockSession.windowBounds = { left: -10, top: -10, width: 800, height: 600 };
232+
await handleLoadSession(mockSession.id);
233+
expect(global.chrome.windows.create).toHaveBeenCalledWith({
234+
url: ['https://example.com', 'https://test.com'],
235+
height: EXPECTED_FALLBACK_BOUNDS.height,
236+
width: EXPECTED_FALLBACK_BOUNDS.width,
237+
top: EXPECTED_FALLBACK_BOUNDS.top,
238+
left: EXPECTED_FALLBACK_BOUNDS.left
239+
});
240+
});
241+
242+
test('should fallback if session bounds extend beyond display', async () => {
243+
mockSession.windowBounds = { left: 100, top: 100, width: 2000, height: 1200 };
244+
await handleLoadSession(mockSession.id);
245+
expect(global.chrome.windows.create).toHaveBeenCalledWith({
246+
url: ['https://example.com', 'https://test.com'],
247+
height: EXPECTED_FALLBACK_BOUNDS.height,
248+
width: EXPECTED_FALLBACK_BOUNDS.width,
249+
top: EXPECTED_FALLBACK_BOUNDS.top,
250+
left: EXPECTED_FALLBACK_BOUNDS.left
251+
});
252+
});
253+
254+
test('should use bounds if they fit exactly within display', async () => {
255+
mockSession.windowBounds = { left: 0, top: 0, width: 1920, height: 1080 };
256+
await handleLoadSession(mockSession.id);
257+
expect(global.chrome.windows.create).toHaveBeenCalledWith({
258+
url: ['https://example.com', 'https://test.com'],
259+
height: 1080,
260+
width: 1920,
261+
top: 0,
262+
left: 0
263+
});
264+
});
265+
266+
test('should fallback if bounds are just barely outside display', async () => {
267+
mockSession.windowBounds = { left: 0, top: 0, width: 1921, height: 1081 };
268+
await handleLoadSession(mockSession.id);
269+
expect(global.chrome.windows.create).toHaveBeenCalledWith({
270+
url: ['https://example.com', 'https://test.com'],
271+
height: EXPECTED_FALLBACK_BOUNDS.height,
272+
width: EXPECTED_FALLBACK_BOUNDS.width,
273+
top: EXPECTED_FALLBACK_BOUNDS.top,
274+
left: EXPECTED_FALLBACK_BOUNDS.left
225275
});
226276
});
227277
});

0 commit comments

Comments
 (0)