From af5f4f97d7f38dbb201e0f09d1b000de6f0c92cb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kristijan=20Ribari=C4=87?= <kriba13@gmail.com>
Date: Sun, 3 Nov 2024 20:22:35 +0100
Subject: [PATCH] feat: Separate pinned tab initialization and handling

This commit refactors the `ZenPinnedTabManager` class to separate the initialization of pinned tabs from the handling of pinned tab events.

- **Initialization:** The `initTabs()` method is now responsible for initializing the pinned tab storage, waiting for the session store to be initialized, and then refreshing the pinned tabs.
- **Event handling:** The `init()` method focuses on setting up the event listeners and observers for pinned tabs.

This separation improves the code structure and allows for better handling of pinned tab initialization, which was previously problematic due to the asynchronous nature of the process.

Additionally, this commit includes minor fixes and improvements:

-  Removed the `TabClose` event handler, as it is currently causing issues with the tab deletion process.
-  Added a check to prevent duplicate pinned tab attributes.
-  Improved the way favicons are set for pinned tabs.
---
 src/ZenPinnedTabManager.mjs | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/src/ZenPinnedTabManager.mjs b/src/ZenPinnedTabManager.mjs
index 7a424d4..7f22e77 100644
--- a/src/ZenPinnedTabManager.mjs
+++ b/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,12 @@
       this._zenClickEventListener = this._onTabClick.bind(this);
     }
 
+    async initTabs() {
+      await ZenPinnedTabsStorage.init();
+      await SessionStore.promiseInitialized;
+      await  this._refreshPinnedTabs();
+    }
+
     async _refreshPinnedTabs() {
       await this._initializePinsCache();
       this._initializePinnedTabs();
@@ -139,9 +142,10 @@
         });
 
         // Set the favicon from cache
-        if (pin.iconUrl) {
-          gBrowser.setIcon(newTab, pin.iconUrl, null,
-              Services.scriptSecurityManager.getSystemPrincipal());
+        if (!!pin.iconUrl) {
+          // TODO: Figure out if there is a better way -
+          //  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 +179,10 @@
             delete tab._zenClickEventListener;
           }
           break;
-        case "TabClose":
-          this._removePinnedAttributes(tab);
-          break;
+          // TODO: Do this in a better way. Closing a second window could trigger remove tab and delete it from db
+        // case "TabClose":
+        //   this._removePinnedAttributes(tab);
+        //   break;
         default:
           console.warn('ZenPinnedTabManager: Unhandled tab event', action);
           break;
@@ -228,6 +233,11 @@
     }
 
     async _setPinnedAttributes(tab) {
+
+      if (tab.hasAttribute("zen-pin-id")) {
+        return;
+      }
+
       const browser = tab.linkedBrowser;
 
       const uuid = gZenUIManager.generateUuidv4();