From 24d7af3da0ed598162fd075a65f87fb075b98016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristijan=20Ribari=C4=87?= Date: Wed, 9 Oct 2024 18:14:09 +0200 Subject: [PATCH 1/3] Fix: Ensure tab context works after moving a tab to different workspace. Calling to _workspaces() always gets the fresh data from db, moved caching responsibility to db for getWorkspaces(). --- src/ZenWorkspaces.mjs | 27 +++++++++++++-------------- src/ZenWorkspacesStorage.mjs | 2 +- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/ZenWorkspaces.mjs b/src/ZenWorkspaces.mjs index 6dbf538..af00a3c 100644 --- a/src/ZenWorkspaces.mjs +++ b/src/ZenWorkspaces.mjs @@ -72,24 +72,23 @@ var ZenWorkspaces = new (class extends ZenMultiWindowFeature { } async _workspaces() { - if (!this._workspaceCache) { - this._workspaceCache = { workspaces: await ZenWorkspacesStorage.getWorkspaces() }; - // Get the active workspace ID from preferences - const activeWorkspaceId = Services.prefs.getStringPref('zen.workspaces.active', ''); + this._workspaceCache = { workspaces: await ZenWorkspacesStorage.getWorkspaces() }; + // Get the active workspace ID from preferences + const activeWorkspaceId = Services.prefs.getStringPref('zen.workspaces.active', ''); - if (activeWorkspaceId) { - const activeWorkspace = this._workspaceCache.workspaces.find((w) => w.uuid === activeWorkspaceId); - // Set the active workspace ID to the first one if the one with selected id doesn't exist - if (!activeWorkspace) { - Services.prefs.setStringPref('zen.workspaces.active', this._workspaceCache.workspaces[0]?.uuid); - } - } else { - // Set the active workspace ID to the first one if active workspace doesn't exist + if (activeWorkspaceId) { + const activeWorkspace = this._workspaceCache.workspaces.find((w) => w.uuid === activeWorkspaceId); + // Set the active workspace ID to the first one if the one with selected id doesn't exist + if (!activeWorkspace) { Services.prefs.setStringPref('zen.workspaces.active', this._workspaceCache.workspaces[0]?.uuid); } - // sort by position - this._workspaceCache.workspaces.sort((a, b) => (a.position ?? Infinity) - (b.position ?? Infinity)); + } else { + // Set the active workspace ID to the first one if active workspace doesn't exist + Services.prefs.setStringPref('zen.workspaces.active', this._workspaceCache.workspaces[0]?.uuid); } + // sort by position + this._workspaceCache.workspaces.sort((a, b) => (a.position ?? Infinity) - (b.position ?? Infinity)); + return this._workspaceCache; } diff --git a/src/ZenWorkspacesStorage.mjs b/src/ZenWorkspacesStorage.mjs index 2dcbd93..2926b47 100644 --- a/src/ZenWorkspacesStorage.mjs +++ b/src/ZenWorkspacesStorage.mjs @@ -150,7 +150,7 @@ var ZenWorkspacesStorage = { async getWorkspaces() { const db = await PlacesUtils.promiseDBConnection(); - const rows = await db.execute(` + const rows = await db.executeCached(` SELECT * FROM zen_workspaces ORDER BY created_at ASC `); return rows.map((row) => ({ From 8ca8157e617ae0ec89d831223172c3913678f4c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristijan=20Ribari=C4=87?= Date: Wed, 9 Oct 2024 19:38:06 +0200 Subject: [PATCH 2/3] feat: Improve workspace ordering and changes tracking This commit introduces improvements to workspace ordering and change tracking in the Zen Workspaces system: - **Workspace Ordering:** - Uses `REAL` data type for `position` in `zen_workspaces` table, allowing for more precise and efficient ordering. - Introduces a new `updateWorkspaceOrder` method to update workspace positions. - Reorders workspaces with large increments to avoid frequent reordering, ensuring consistent ordering after changes. - Implements a mechanism to check for necessary reordering after position updates and reorder all workspaces if required. - **Changes Tracking:** - Tracks changes to workspaces by inserting a record in `zen_workspaces_changes` for each modified workspace. - Adds an index on `uuid` column in `zen_workspaces_changes` table for faster querying. - Updates `getLastChangeTimestamp` method to return the last change timestamp from the changes tracking table, providing accurate timestamp for changes. These changes optimize workspace management, improve accuracy of ordering, and enhance the performance of change tracking. --- src/ZenWorkspaces.mjs | 30 +++-- src/ZenWorkspacesStorage.mjs | 213 +++++++++++++++++++---------------- 2 files changed, 137 insertions(+), 106 deletions(-) diff --git a/src/ZenWorkspaces.mjs b/src/ZenWorkspaces.mjs index af00a3c..2e427eb 100644 --- a/src/ZenWorkspaces.mjs +++ b/src/ZenWorkspaces.mjs @@ -30,15 +30,19 @@ var ZenWorkspaces = new (class extends ZenMultiWindowFeature { Services.obs.addObserver(this, "weave:engine:sync:finish"); } - observe(subject, topic, data) { + async observe(subject, topic, data) { if (topic === "weave:engine:sync:finish" && data === "workspaces") { - this._workspaceCache = null; // Clear cache to fetch fresh data - this.updateWorkspaceStrip(); - } - } + try { + const lastChangeTimestamp = await ZenWorkspacesStorage.getLastChangeTimestamp(); - updateWorkspaceStrip() { - this._propagateWorkspaceData().catch(console.error); + if (!this._workspaceCache || !this._workspaceCache.lastChangeTimestamp || lastChangeTimestamp > this._workspaceCache.lastChangeTimestamp) { + this._workspaceCache = null; + await this._propagateWorkspaceData(); + } + } catch (error) { + console.error("Error updating workspaces after sync:", error); + } + } } get shouldHaveWorkspaces() { @@ -72,7 +76,16 @@ var ZenWorkspaces = new (class extends ZenMultiWindowFeature { } async _workspaces() { - this._workspaceCache = { workspaces: await ZenWorkspacesStorage.getWorkspaces() }; + if (this._workspaceCache) { + return this._workspaceCache; + } + + const [workspaces, lastChangeTimestamp] = await Promise.all([ + ZenWorkspacesStorage.getWorkspaces(), + ZenWorkspacesStorage.getLastChangeTimestamp() + ]); + + this._workspaceCache = { workspaces, lastChangeTimestamp }; // Get the active workspace ID from preferences const activeWorkspaceId = Services.prefs.getStringPref('zen.workspaces.active', ''); @@ -831,6 +844,7 @@ var ZenWorkspaces = new (class extends ZenMultiWindowFeature { delete this._lastSelectedWorkspaceTabs[previousWorkspaceID]; } } + this._workspaceCache = null; const workspaces = await this._workspaces(); await this.changeWorkspace(workspaces.workspaces.find((workspace) => workspace.uuid === workspaceID)); } diff --git a/src/ZenWorkspacesStorage.mjs b/src/ZenWorkspacesStorage.mjs index 2926b47..fec293a 100644 --- a/src/ZenWorkspacesStorage.mjs +++ b/src/ZenWorkspacesStorage.mjs @@ -15,12 +15,17 @@ var ZenWorkspacesStorage = { icon TEXT, is_default INTEGER NOT NULL DEFAULT 0, container_id INTEGER, - position INTEGER NOT NULL DEFAULT 0, + position REAL NOT NULL DEFAULT 0, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL ) `); + // Create an index on the uuid column + await db.execute(` + CREATE INDEX IF NOT EXISTS idx_zen_workspaces_uuid ON zen_workspaces(uuid) + `); + // Create the changes tracking table if it doesn't exist await db.execute(` CREATE TABLE IF NOT EXISTS zen_workspaces_changes ( @@ -28,6 +33,11 @@ var ZenWorkspacesStorage = { timestamp INTEGER NOT NULL ) `); + + // Create an index on the uuid column for changes tracking table + await db.execute(` + CREATE INDEX IF NOT EXISTS idx_zen_workspaces_changes_uuid ON zen_workspaces_changes(uuid) + `); }); }, @@ -63,62 +73,38 @@ var ZenWorkspacesStorage = { const changedUUIDs = new Set(); await PlacesUtils.withConnectionWrapper('ZenWorkspacesStorage.saveWorkspace', async (db) => { - const now = Date.now(); + await db.executeTransaction(async () => { + const now = Date.now(); - await db.executeTransaction(async function() { - // If the workspace is set as default, unset is_default for all other workspaces + // Handle default workspace if (workspace.default) { await db.execute(`UPDATE zen_workspaces SET is_default = 0 WHERE uuid != :uuid`, { uuid: workspace.uuid }); - - // Collect UUIDs of workspaces that were unset as default const unsetDefaultRows = await db.execute(`SELECT uuid FROM zen_workspaces WHERE is_default = 0 AND uuid != :uuid`, { uuid: workspace.uuid }); for (const row of unsetDefaultRows) { changedUUIDs.add(row.getResultByName('uuid')); } } - // Get the current maximum position - const maxOrderResult = await db.execute(`SELECT MAX("position") as max_position FROM zen_workspaces`); - const maxOrder = maxOrderResult[0].getResultByName('max_position') || 0; - - let newOrder; - - if ('position' in workspace && workspace.position !== null && Number.isInteger(workspace.position)) { - // If position is provided, check if it's already occupied - const occupiedOrderResult = await db.execute(` - SELECT uuid FROM zen_workspaces WHERE "position" = :position AND uuid != :uuid - `, { position: workspace.position, uuid: workspace.uuid }); - - if (occupiedOrderResult.length > 0) { - // If the position is occupied, shift the positions of subsequent workspaces - await db.execute(` - UPDATE zen_workspaces - SET "position" = "position" + 1 - WHERE "position" >= :position AND uuid != :uuid - `, { position: workspace.position, uuid: workspace.uuid }); - - // Collect UUIDs of workspaces whose positions were shifted - for (const row of occupiedOrderResult) { - changedUUIDs.add(row.getResultByName('uuid')); - } - } - - newOrder = workspace.position; + let newPosition; + if ('position' in workspace && Number.isFinite(workspace.position)) { + newPosition = workspace.position; } else { - // If no position is provided, set it to the last position - newOrder = maxOrder + 1; + // Get the maximum position + const maxPositionResult = await db.execute(`SELECT MAX("position") as max_position FROM zen_workspaces`); + const maxPosition = maxPositionResult[0].getResultByName('max_position') || 0; + newPosition = maxPosition + 1000; // Add a large increment to avoid frequent reordering } // Insert or replace the workspace await db.executeCached(` INSERT OR REPLACE INTO zen_workspaces ( - uuid, name, icon, is_default, container_id, created_at, updated_at, "position" - ) VALUES ( - :uuid, :name, :icon, :is_default, :container_id, - COALESCE((SELECT created_at FROM zen_workspaces WHERE uuid = :uuid), :now), - :now, - :position - ) + uuid, name, icon, is_default, container_id, created_at, updated_at, "position" + ) VALUES ( + :uuid, :name, :icon, :is_default, :container_id, + COALESCE((SELECT created_at FROM zen_workspaces WHERE uuid = :uuid), :now), + :now, + :position + ) `, { uuid: workspace.uuid, name: workspace.name, @@ -126,20 +112,21 @@ var ZenWorkspacesStorage = { is_default: workspace.default ? 1 : 0, container_id: workspace.containerTabId || null, now, - position: newOrder + position: newPosition }); - // Record the change in the changes tracking table + // Record the change await db.execute(` INSERT OR REPLACE INTO zen_workspaces_changes (uuid, timestamp) - VALUES (:uuid, :timestamp) + VALUES (:uuid, :timestamp) `, { uuid: workspace.uuid, - timestamp: Math.floor(now / 1000) // Unix timestamp in seconds + timestamp: Math.floor(now / 1000) }); - // Add the main workspace UUID to the changed set changedUUIDs.add(workspace.uuid); + + await this.updateLastChangeTimestamp(db); }); }); @@ -183,6 +170,8 @@ var ZenWorkspacesStorage = { uuid, timestamp: Math.floor(now / 1000) }); + + await this.updateLastChangeTimestamp(db); }); if (notifyObservers) { @@ -194,6 +183,7 @@ var ZenWorkspacesStorage = { await PlacesUtils.withConnectionWrapper('ZenWorkspacesStorage.wipeAllWorkspaces', async (db) => { await db.execute(`DELETE FROM zen_workspaces`); await db.execute(`DELETE FROM zen_workspaces_changes`); + await this.updateLastChangeTimestamp(db); }); }, @@ -201,7 +191,7 @@ var ZenWorkspacesStorage = { const changedUUIDs = []; await PlacesUtils.withConnectionWrapper('ZenWorkspacesStorage.setDefaultWorkspace', async (db) => { - await db.executeTransaction(async function () { + await db.executeTransaction(async () => { const now = Date.now(); // Unset the default flag for all other workspaces await db.execute(`UPDATE zen_workspaces SET is_default = 0 WHERE uuid != :uuid`, { uuid }); @@ -226,6 +216,8 @@ var ZenWorkspacesStorage = { // Add the main workspace UUID to the changed set changedUUIDs.push(uuid); + + await this.updateLastChangeTimestamp(db); }); }); @@ -265,79 +257,104 @@ var ZenWorkspacesStorage = { }); }, - async updateWorkspaceOrder(uuid, newOrder, notifyObservers = true) { - const changedUUIDs = [uuid]; + async updateWorkspaceOrder(uuid, newPosition, notifyObservers = true) { + const changedUUIDs = new Set([uuid]); await PlacesUtils.withConnectionWrapper('ZenWorkspacesStorage.updateWorkspaceOrder', async (db) => { - await db.executeTransaction(async function () { - // Get the current position of the workspace - const currentOrderResult = await db.execute(` - SELECT "position" FROM zen_workspaces WHERE uuid = :uuid - `, { uuid }); - const currentOrder = currentOrderResult[0].getResultByName('position'); + await db.executeTransaction(async () => { + const now = Date.now(); - if (currentOrder === newOrder) { - return; // No change needed - } + // Get the positions of the workspaces before and after the new position + const neighborPositions = await db.execute(` + SELECT + (SELECT MAX("position") FROM zen_workspaces WHERE "position" < :newPosition) as before_position, + (SELECT MIN("position") FROM zen_workspaces WHERE "position" > :newPosition) as after_position + `, { newPosition }); - if (newOrder > currentOrder) { - // Moving down: decrement position of workspaces between old and new positions - const rows = await db.execute(` - SELECT uuid FROM zen_workspaces - WHERE "position" > :currentOrder AND "position" <= :newOrder - `, { currentOrder, newOrder }); + let beforePosition = neighborPositions[0].getResultByName('before_position'); + let afterPosition = neighborPositions[0].getResultByName('after_position'); - await db.execute(` - UPDATE zen_workspaces - SET "position" = "position" - 1 - WHERE "position" > :currentOrder AND "position" <= :newOrder - `, { currentOrder, newOrder }); - - for (const row of rows) { - changedUUIDs.push(row.getResultByName('uuid')); - } + // Calculate the new position + if (beforePosition === null && afterPosition === null) { + // This is the only workspace + newPosition = 1000; + } else if (beforePosition === null) { + // This will be the first workspace + newPosition = afterPosition / 2; + } else if (afterPosition === null) { + // This will be the last workspace + newPosition = beforePosition + 1000; } else { - // Moving up: increment position of workspaces between new and old positions - const rows = await db.execute(` - SELECT uuid FROM zen_workspaces - WHERE "position" >= :newOrder AND "position" < :currentOrder - `, { currentOrder, newOrder }); - - await db.execute(` - UPDATE zen_workspaces - SET "position" = "position" + 1 - WHERE "position" >= :newOrder AND "position" < :currentOrder - `, { currentOrder, newOrder }); - - for (const row of rows) { - changedUUIDs.push(row.getResultByName('uuid')); - } + // This workspace will be between two others + newPosition = (beforePosition + afterPosition) / 2; } - // Set the new position for the workspace + // Update the workspace's position await db.execute(` UPDATE zen_workspaces - SET "position" = :newOrder + SET "position" = :newPosition WHERE uuid = :uuid - `, { uuid, newOrder }); + `, { uuid, newPosition }); - // Record the change for the specified workspace - const now = Date.now(); + // Record the change await db.execute(` INSERT OR REPLACE INTO zen_workspaces_changes (uuid, timestamp) - VALUES (:uuid, :timestamp) + VALUES (:uuid, :timestamp) `, { uuid, timestamp: Math.floor(now / 1000) }); - // Add the main workspace UUID to the changed set - changedUUIDs.push(uuid); + await this.updateLastChangeTimestamp(db); + + // Check if reordering is necessary + if (this.shouldReorderWorkspaces(beforePosition, newPosition, afterPosition)) { + await this.reorderAllWorkspaces(db, changedUUIDs); + } }); }); if (notifyObservers) { - this._notifyWorkspacesChanged("zen-workspace-updated", changedUUIDs); + this._notifyWorkspacesChanged("zen-workspace-updated", Array.from(changedUUIDs)); } }, + + shouldReorderWorkspaces(before, current, after) { + const minGap = 0.001; // Minimum allowed gap between positions + return (before !== null && current - before < minGap) || (after !== null && after - current < minGap); + }, + + async reorderAllWorkspaces(db, changedUUIDs) { + const workspaces = await db.execute(` + SELECT uuid + FROM zen_workspaces + ORDER BY "position" ASC + `); + + for (let i = 0; i < workspaces.length; i++) { + const newPosition = (i + 1) * 1000; // Use large increments + await db.execute(` + UPDATE zen_workspaces + SET "position" = :newPosition + WHERE uuid = :uuid + `, { newPosition, uuid: workspaces[i].getResultByName('uuid') }); + changedUUIDs.add(workspaces[i].getResultByName('uuid')); + } + }, + + async updateLastChangeTimestamp(db) { + const now = Date.now(); + await db.execute(` + INSERT OR REPLACE INTO moz_meta (key, value) + VALUES ('zen_workspaces_last_change', :now) + `, { now }); + }, + + async getLastChangeTimestamp() { + const db = await PlacesUtils.promiseDBConnection(); + const result = await db.executeCached(` + SELECT value FROM moz_meta WHERE key = 'zen_workspaces_last_change' + `); + return result.length ? parseInt(result[0].getResultByName('value'), 10) : 0; + }, }; From b296405a45ccf99dabb34b2b1726a843809d556c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristijan=20Ribari=C4=87?= Date: Wed, 9 Oct 2024 19:49:45 +0200 Subject: [PATCH 3/3] Fix: Use integer positions for workspaces This commit changes the `position` column in the `workspaces` table to be an `INTEGER` instead of a `REAL` data type. This change is made to ensure that workspace positions are always whole numbers, which prevents issues with floating-point precision and ensures consistent ordering of workspaces. Additionally, it updates the logic for calculating new workspace positions to use `Math.floor` to round down to the nearest integer, ensuring that positions are always integers. This change also adjusts the `minGap` constant used in the `shouldReorderWorkspaces` function to `1`, which reflects the minimum allowed gap between workspace positions now that they are integers. --- src/ZenWorkspacesStorage.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ZenWorkspacesStorage.mjs b/src/ZenWorkspacesStorage.mjs index fec293a..caf62b0 100644 --- a/src/ZenWorkspacesStorage.mjs +++ b/src/ZenWorkspacesStorage.mjs @@ -15,7 +15,7 @@ var ZenWorkspacesStorage = { icon TEXT, is_default INTEGER NOT NULL DEFAULT 0, container_id INTEGER, - position REAL NOT NULL DEFAULT 0, + position INTEGER NOT NULL DEFAULT 0, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL ) @@ -280,13 +280,13 @@ var ZenWorkspacesStorage = { newPosition = 1000; } else if (beforePosition === null) { // This will be the first workspace - newPosition = afterPosition / 2; + newPosition = Math.floor(afterPosition / 2); } else if (afterPosition === null) { // This will be the last workspace newPosition = beforePosition + 1000; } else { // This workspace will be between two others - newPosition = (beforePosition + afterPosition) / 2; + newPosition = Math.floor((beforePosition + afterPosition) / 2); } // Update the workspace's position @@ -320,7 +320,7 @@ var ZenWorkspacesStorage = { }, shouldReorderWorkspaces(before, current, after) { - const minGap = 0.001; // Minimum allowed gap between positions + const minGap = 1; // Minimum allowed gap between positions return (before !== null && current - before < minGap) || (after !== null && after - current < minGap); },