Merge pull request #75 from kristijanribaric/fix/prevent-infinite-loop-when-pinning-a-tab

Fix/prevent infinite loop when pinning a tab

src/ZenPinnedTabManager.mjs

@@ -36,13 +36,10 @@
     }
   }
 
-  class ZenPinnedTabManager extends ZenMultiWindowFeature {
+  class ZenPinnedTabManager extends ZenDOMOperatedFeature {
 
-    async init() {
+    init() {
       this.observer = new ZenPinnedTabsObserver();
-      await ZenPinnedTabsStorage.init();
-      await SessionStore.promiseInitialized;
-      await  this._refreshPinnedTabs();
       this._initClosePinnedTabShortcut();
       this._insertItemsIntoTabContextMenu();
       this.observer.addPinnedTabListener(this._onPinnedTabEvent.bind(this));
@@ -50,6 +47,11 @@
       this._zenClickEventListener = this._onTabClick.bind(this);
     }
 
+    async initTabs() {
+      await ZenPinnedTabsStorage.init();
+      await  this._refreshPinnedTabs();
+    }
+
     async _refreshPinnedTabs() {
       await this._initializePinsCache();
       this._initializePinnedTabs();
@@ -139,9 +141,10 @@
         });
 
         // Set the favicon from cache
-        if (pin.iconUrl) {
+        if (!!pin.iconUrl) {
-          gBrowser.setIcon(newTab, pin.iconUrl, null,
+          // TODO: Figure out if there is a better way -
-              Services.scriptSecurityManager.getSystemPrincipal());
+          //  calling gBrowser.setIcon messes shit up and should be avoided. I think this works for now.
+          newTab.setAttribute("image", pin.iconUrl);
         }
 
         newTab.setAttribute("zen-pin-id", pin.uuid);
@@ -175,9 +178,10 @@
             delete tab._zenClickEventListener;
           }
           break;
-        case "TabClose":
+          // TODO: Do this in a better way. Closing a second window could trigger remove tab and delete it from db
-          this._removePinnedAttributes(tab);
+        // case "TabClose":
-          break;
+        //   this._removePinnedAttributes(tab);
+        //   break;
         default:
           console.warn('ZenPinnedTabManager: Unhandled tab event', action);
           break;
@@ -228,6 +232,11 @@
     }
 
     async _setPinnedAttributes(tab) {
+
+      if (tab.hasAttribute("zen-pin-id")) {
+        return;
+      }
+
       const browser = tab.linkedBrowser;
 
       const uuid = gZenUIManager.generateUuidv4();

src/ZenPinnedTabsStorage.mjs

@@ -1,11 +1,7 @@
 var ZenPinnedTabsStorage = {
   async init() {
     console.log('ZenPinnedTabsStorage: Initializing...');
-    try {
+    await this._ensureTable();
-      await this._ensureTable();
-    } catch (e) {
-      console.warn('ZenPinnedTabsStorage: Failed to initialize', e);
-    }
   },
 
   async _ensureTable() {
@@ -13,23 +9,32 @@ var ZenPinnedTabsStorage = {
       // Create the pins table if it doesn't exist
       await db.execute(`
         CREATE TABLE IF NOT EXISTS zen_pins (
-          id INTEGER PRIMARY KEY,
+      id INTEGER PRIMARY KEY,
-          uuid TEXT UNIQUE NOT NULL,
+      uuid TEXT UNIQUE NOT NULL,
-          title TEXT NOT NULL,
+      title TEXT NOT NULL,
-          url TEXT NOT NULL,
+      url TEXT,
-          container_id INTEGER,
+      container_id INTEGER,
-          workspace_uuid TEXT,
+      workspace_uuid TEXT,
-          position INTEGER NOT NULL DEFAULT 0,
+      position INTEGER NOT NULL DEFAULT 0,
-          created_at INTEGER NOT NULL,
+      is_essential BOOLEAN NOT NULL DEFAULT 0,
-          updated_at INTEGER NOT NULL
+      is_group BOOLEAN NOT NULL DEFAULT 0,
-        )
+      parent_uuid TEXT,
+      created_at INTEGER NOT NULL,
+      updated_at INTEGER NOT NULL,
+      FOREIGN KEY (parent_uuid) REFERENCES zen_pins(uuid) ON DELETE SET NULL
+          )
       `);
 
-      // Create an index on the uuid column
+
+      // Create indices
       await db.execute(`
         CREATE INDEX IF NOT EXISTS idx_zen_pins_uuid ON zen_pins(uuid)
       `);
 
+      await db.execute(`
+        CREATE INDEX IF NOT EXISTS idx_zen_pins_parent_uuid ON zen_pins(parent_uuid)
+      `);
+
       // Create the changes tracking table if it doesn't exist
       await db.execute(`
         CREATE TABLE IF NOT EXISTS zen_pins_changes (
@@ -70,33 +75,40 @@ var ZenPinnedTabsStorage = {
         if ('position' in pin && Number.isFinite(pin.position)) {
           newPosition = pin.position;
         } else {
-          // Get the maximum position
+          // Get the maximum position within the same parent group (or null for root level)
-          const maxPositionResult = await db.execute(`SELECT MAX("position") as max_position FROM zen_pins`);
+          const maxPositionResult = await db.execute(`
+            SELECT MAX("position") as max_position
+            FROM zen_pins
+            WHERE COALESCE(parent_uuid, '') = COALESCE(:parent_uuid, '')
+          `, { parent_uuid: pin.parentUuid || null });
           const maxPosition = maxPositionResult[0].getResultByName('max_position') || 0;
-          newPosition = maxPosition + 1000; // Add a large increment to avoid frequent reordering
+          newPosition = maxPosition + 1000;
         }
 
         // Insert or replace the pin
         await db.executeCached(`
           INSERT OR REPLACE INTO zen_pins (
             uuid, title, url, container_id, workspace_uuid, position,
-            created_at, updated_at
+            is_essential, is_group, parent_uuid, created_at, updated_at
           ) VALUES (
             :uuid, :title, :url, :container_id, :workspace_uuid, :position,
+            :is_essential, :is_group, :parent_uuid,
             COALESCE((SELECT created_at FROM zen_pins WHERE uuid = :uuid), :now),
             :now
           )
         `, {
           uuid: pin.uuid,
           title: pin.title,
-          url: pin.url,
+          url: pin.isGroup ? null : pin.url,
           container_id: pin.containerTabId || null,
           workspace_uuid: pin.workspaceUuid || null,
           position: newPosition,
+          is_essential: pin.isEssential || false,
+          is_group: pin.isGroup || false,
+          parent_uuid: pin.parentUuid || null,
           now
         });
 
-        // Record the change
         await db.execute(`
           INSERT OR REPLACE INTO zen_pins_changes (uuid, timestamp)
           VALUES (:uuid, :timestamp)
@@ -106,7 +118,6 @@ var ZenPinnedTabsStorage = {
         });
 
         changedUUIDs.add(pin.uuid);
-
         await this.updateLastChangeTimestamp(db);
       });
     });
@@ -119,7 +130,8 @@ var ZenPinnedTabsStorage = {
   async getPins() {
     const db = await PlacesUtils.promiseDBConnection();
     const rows = await db.executeCached(`
-      SELECT * FROM zen_pins ORDER BY position ASC
+      SELECT * FROM zen_pins
+      ORDER BY parent_uuid NULLS FIRST, position ASC
     `);
     return rows.map((row) => ({
       uuid: row.getResultByName('uuid'),
@@ -127,7 +139,31 @@ var ZenPinnedTabsStorage = {
       url: row.getResultByName('url'),
       containerTabId: row.getResultByName('container_id'),
       workspaceUuid: row.getResultByName('workspace_uuid'),
-      position: row.getResultByName('position')
+      position: row.getResultByName('position'),
+      isEssential: Boolean(row.getResultByName('is_essential')),
+      isGroup: Boolean(row.getResultByName('is_group')),
+      parentUuid: row.getResultByName('parent_uuid')
+    }));
+  },
+
+  async getGroupChildren(groupUuid) {
+    const db = await PlacesUtils.promiseDBConnection();
+    const rows = await db.executeCached(`
+      SELECT * FROM zen_pins 
+      WHERE parent_uuid = :groupUuid 
+      ORDER BY position ASC
+    `, { groupUuid });
+
+    return rows.map((row) => ({
+      uuid: row.getResultByName('uuid'),
+      title: row.getResultByName('title'),
+      url: row.getResultByName('url'),
+      containerTabId: row.getResultByName('container_id'),
+      workspaceUuid: row.getResultByName('workspace_uuid'),
+      position: row.getResultByName('position'),
+      isEssential: Boolean(row.getResultByName('is_essential')),
+      isGroup: Boolean(row.getResultByName('is_group')),
+      parentUuid: row.getResultByName('parent_uuid')
     }));
   },
 
@@ -135,22 +171,44 @@ var ZenPinnedTabsStorage = {
     const changedUUIDs = [uuid];
 
     await PlacesUtils.withConnectionWrapper('ZenPinnedTabsStorage.removePin', async (db) => {
-      await db.execute(
+      await db.executeTransaction(async () => {
-          `DELETE FROM zen_pins WHERE uuid = :uuid`,
+        // Get all child UUIDs first for change tracking
-          { uuid }
+        const children = await db.execute(
-      );
+            `SELECT uuid FROM zen_pins WHERE parent_uuid = :uuid`,
+            { uuid }
+        );
 
-      // Record the removal as a change
+        // Add child UUIDs to changedUUIDs array
-      const now = Date.now();
+        for (const child of children) {
-      await db.execute(`
+          changedUUIDs.push(child.getResultByName('uuid'));
-        INSERT OR REPLACE INTO zen_pins_changes (uuid, timestamp)
+        }
-        VALUES (:uuid, :timestamp)
+
-      `, {
+        // Delete all children in a single statement
-        uuid,
+        await db.execute(
-        timestamp: Math.floor(now / 1000)
+            `DELETE FROM zen_pins WHERE parent_uuid = :uuid`,
+            { uuid }
+        );
+
+        // Delete the pin/group itself
+        await db.execute(
+            `DELETE FROM zen_pins WHERE uuid = :uuid`,
+            { uuid }
+        );
+
+        // Record the changes
+        const now = Math.floor(Date.now() / 1000);
+        for (const changedUuid of changedUUIDs) {
+          await db.execute(`
+            INSERT OR REPLACE INTO zen_pins_changes (uuid, timestamp)
+            VALUES (:uuid, :timestamp)
+          `, {
+            uuid: changedUuid,
+            timestamp: now
+          });
+        }
+
+        await this.updateLastChangeTimestamp(db);
       });
-
-      await this.updateLastChangeTimestamp(db);
     });
 
     if (notifyObservers) {