Made browser actions per tab on Safari

safari/ext/background.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 63 matching lines @@
     }
   };
 
 
   /* Tabs */
 
   Tab = function(tab)
   {
     this._tab = tab;
 
+    this.browserAction = new BrowserAction(this);
+
     this.onLoading = new LoadingTabEventTarget(tab);
     this.onBeforeNavigate = new TabEventTarget(tab, "beforeNavigate", false);
     this.onCompleted = new TabEventTarget(tab, "navigate", false);
     this.onActivated = new TabEventTarget(tab, "activate", false);
     this.onRemoved = new TabEventTarget(tab, "close", false);
   };
   Tab.prototype = {
     get url()
     {
       return this._tab.url;
     },
     close: function()
     {
       this._tab.close();
     },
     activate: function()
     {
       this._tab.activate();
     },
     sendMessage: function(message, responseCallback)
     {
       _sendMessage(
         message, responseCallback,
         this._tab.page, this._tab
       );
-    },
-    browserAction: {
-      setIcon: function(path)
-      {
-        safari.extension.toolbarItems[0].image = safari.extension.baseURI + path;
-      },
-      setTitle: function(title)
-      {
-        safari.extension.toolbarItems[0].toolTip = title;
-      },
-      setBadge: function(badge)
-      {
-        if (!badge)
-          safari.extension.toolbarItems[0].badge = 0;
-        else if ("number" in badge)
-          safari.extension.toolbarItems[0].badge = badge.number;
-      }
     }
   };
 
   TabMap = function(deleteTabOnBeforeNavigate)
   {
     this._tabs = [];
     this._values = [];
 
     this._deleteOnEvent = this._deleteOnEvent.bind(this);
     this._deleteTabOnBeforeNavigate = deleteTabOnBeforeNavigate;
@@ skipping 50 matching lines @@
     {
       // delay so that other event handlers can still lookup this tab
       setTimeout(this._delete.bind(this, event.target), 0);
     }
   };
   TabMap.prototype["delete"] = function(tab)
   {
     this._delete(tab._tab);
   };
 
+  ext.tabs = {
+    onLoading: new LoadingTabEventTarget(safari.application),
+    onBeforeNavigate: new TabEventTarget(safari.application, "beforeNavigate", true),
+    onCompleted: new TabEventTarget(safari.application, "navigate", true),
+    onActivated: new TabEventTarget(safari.application, "activate", true),
+    onRemoved: new TabEventTarget(safari.application, "close", true)
+  };
+
+
+  /* Browser actions */
+
+  var globalTooltipItemProperties = {}
+  var tooltipItemPropertiesPerTab = new TabMap();

[Wladimir Palant, 2014/01/15 15:24:47]

You meant toolbarItem, not tooltipItem, right?

[Sebastian Noack, 2014/01/15 19:25:47]

It is gone now.

+
+  var getToolbarItemForWindow = function(win)
+  {
+    for (var i = 0; i < safari.extension.toolbarItems.length; i++)
+    {
+      var toolbarItem = safari.extension.toolbarItems[i];
+
+      if (toolbarItem.browserWindow == win)
+        return toolbarItem;
+    }

[Wladimir Palant, 2014/01/15 15:24:47]

This should return something if no toolbar item is found - return null; at the
end. The callers also need to handle this scenario properly. We *should* not see
this scenario but I don't think the API guarantees that our toolbar item will be
in every window.

[Sebastian Noack, 2014/01/15 19:25:47]

Done. Even though I think that it is a bug when this actually happens, and I
would prefer to see an error in that case.

+  };
+
+  var BrowserAction = function(tab)
+  {
+    this._tab = tab;
+  };
+  BrowserAction.prototype = {
+    _set: function(property, value)
+    {
+      var currentWindow = this._tab._tab.browserWindow;
+      var toolbarItem = getToolbarItemForWindow(currentWindow);
+
+      if (!(property in globalTooltipItemProperties))
+        globalTooltipItemProperties[property] = toolbarItem[property];
+
+      if (this._tab._tab == currentWindow.activeTab)
+        toolbarItem[property] = value;
+
+      var props = tooltipItemPropertiesPerTab.get(this._tab);
+      if (!props)
+      {
+        props = {};
+        tooltipItemPropertiesPerTab.set(this._tab, props);
+      }
+      props[property] = value;
+    },
+    setIcon: function(path)
+    {
+      this._set("image", safari.extension.baseURI + path);
+    },
+    setTitle: function(title)
+    {
+      this._set("toolTip", title);
+    },
+    setBadge: function(badge)
+    {
+      if (!badge)
+        this._set("badge", 0);
+      else if ("number" in badge)
+        this._set("badge", badge.number);
+    }
+  };
+
+  ext.tabs.onActivated.addListener(function(tab)
+  {
+    var props = tooltipItemPropertiesPerTab.get(tab);
+    var toolbarItem = getToolbarItemForWindow(tab._tab.browserWindow);
+
+    for (var property in globalTooltipItemProperties)
+    {
+      if (props && property in props)
+        toolbarItem[property] = props[property];
+      else
+        toolbarItem[property] = globalTooltipItemProperties[property];
+    }

[Wladimir Palant, 2014/01/15 15:24:47]

What's the point of globalTooltipItemProperties? I guess the idea is to show a
proper state for new tabs where nothing loaded yet (no about:blank load
notification in Safari?). But this should really be:

    toolbarItem.badge = 0;
    if (props)
      for (property in props)
        toolbarItem[property] = props[property];

This makes sure the badge is never stuck. As to the other properties, it should
be fine to have those in a "wrong" state for new tabs (your approach doesn't
really prevent it either).

[Sebastian Noack, 2014/01/15 19:25:47]

This approach just emulates the behavior we already have in Chrome, that when
there is no state defined for the current tab, it falls back to the global
state. You might be right that ABP relies on that behavior only for the badge.
However I would like to keep the abstraction layer generic. So ABP-specific code
should be avoided here. But even more important, having different behavior for
Safari and Chrome, would quickly lead to strange bugs, when high level code
changes.

Also the code has changed a lot meanwhile. So I'm not sure whether this
discussion is actually still current, since the new version can also set the
global state via ext.browserAction, and it has to so, in order fix issues on
Safari.

+  });
+
 
   /* Windows */
 
   Window = function(win)
   {
     this._win = win;
   }
   Window.prototype = {
     get visible()
     {
@@ skipping 325 matching lines @@
       {
         return new Window(win);
       }));
     },
     getLastFocused: function(callback)
     {
       callback(new Window(safari.application.activeBrowserWindow));
     }
   };
 
-  ext.tabs = {
-    onLoading: new LoadingTabEventTarget(safari.application),
-    onBeforeNavigate: new TabEventTarget(safari.application, "beforeNavigate", true),
-    onCompleted: new TabEventTarget(safari.application, "navigate", true),
-    onActivated: new TabEventTarget(safari.application, "activate", true),
-    onRemoved: new TabEventTarget(safari.application, "close", true)
-  };
-
   ext.backgroundPage = {
     getWindow: function()
     {
       return safari.extension.globalPage.contentWindow;
     }
   };
 
   ext.onMessage = new BackgroundMessageEventTarget();
 
   // TODO: Implement context menu
   ext.contextMenus = {
     create: function(title, contexts, onclick) {},
     removeAll: function(callback) {}
   };
 })();