Implemented ext.contextMenus for Safari

chrome/background.js

Index: chrome/background.js
===================================================================
--- a/chrome/background.js
+++ b/chrome/background.js
@@ -370,10 +370,11 @@
     handlerBehaviorChanged: chrome.webRequest.handlerBehaviorChanged
   };
 
+  var contextMenu = [];

[Wladimir Palant, 2014/01/17 15:36:55]

Nit: Would contextMenuItems be a better name?

[Thomas Greiner, 2014/01/18 10:32:05]

Done.

   ext.contextMenus = {
-    create: function(title, contexts, onclick)
+    addMenuItem: function(title, contexts, onclick)
     {
-      chrome.contextMenus.create({
+      contextMenu.push({
         title: title,
         contexts: contexts,
         onclick: function(info, tab)
@@ -382,9 +383,29 @@
         }
       });
     },
-    removeAll: function(callback)
+    removeMenuItems: function()
     {
-      chrome.contextMenus.removeAll(callback);
+      contextMenu = [];
+    },
+    showMenu: function()
+    {
+      chrome.contextMenus.removeAll(function()
+      {
+        for (var i = 0; i < contextMenu.length; i++)
+        {
+          var item = contextMenu[i];
+          chrome.contextMenus.create({
+            title: item.title,
+            contexts: item.contexts,
+            onclick: item.onclick
+          });
+        }
+      });
+    },
+    hideMenu: function()

[Wladimir Palant, 2014/01/17 15:36:55]

Nit: These methods should be called showMenuItems/hideMenuItems to indicate that
they affect all menu items.

[Thomas Greiner, 2014/01/18 10:32:05]

Done.

+    {
+      chrome.contextMenus.removeAll();
     }
   };
+
 })();

[Wladimir Palant, 2014/01/17 15:36:55]

The API here is somewhat unexpected - addMenuItem and removeMenuItems have no
immediate effect. I would expect an internal flag variable isContextMenuHidden -
false by default, set in showMenu and hideMenu, same as for Safari. If
isContextMenuHidden is true then addMenuItem calls showMenu and removeMenuItems
calls hideMenu automatically. Also, showMenu can check that flag and return
early if shown is already true - no point wasting effort here. Note that this is
already how it works in Safari.

[Thomas Greiner, 2014/01/18 10:32:05]

Done. However, I decided to go with true as the default value for
isContextMenuHidden because it makes more sense to have it hidden by default and
only show it when something has been added to it.
I also changed this in the Safari implementation to make it consistent.