Implemented ext.contextMenus for Safari

safari/background.js

Index: safari/background.js
===================================================================
--- a/safari/background.js
+++ b/safari/background.js
@@ -523,12 +523,67 @@
 
   ext.onMessage = new MessageEventTarget(safari.application);
 
-  // TODO: Implement context menu
+  var contextMenu = [];
+  var isContextMenuHidden = false;
   ext.contextMenus = {
-    create: function(title, contexts, onclick) {},
-    removeAll: function(callback) {}
+    addMenuItem: function(title, contexts, onclick)
+    {
+      contextMenu.push({
+        id: "block-element", 

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

So all menu items have the same ID?

How about using "context-menu-item-" + contextMenu.length as identifier? I'm
assuming here that menu item IDs have to be unique across all extensions,
otherwise String(contextMenu.length) will be sufficient - and you will be able
to look up the right menu item in the array without looping.

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

Done. According to Apple "The identifier must be unique within your extension."
https://developer.apple.com/library/safari/documentation/Tools/Conceptual/Saf...

+        title: title,
+        item: null,
+        contexts: contexts,
+        onclick: onclick
+      });
+    },
+    removeMenuItems: function()
+    {
+      contextMenu = [];
+    },
+    showMenu: function()
+    {
+      isContextMenuHidden = false;
+    },
+    hideMenu: function()
+    {
+      isContextMenuHidden = true;
+    }
   };
 
+  // Create context menu items
+  safari.application.addEventListener("contextmenu", function(event)
+  {
+    var context = event.userInfo.tagName.toLowerCase();
+    if (context == "img")
+      context = "image";

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

Seeing that srcUrl can be null, it probably makes sense to add one more check
here:

  if (!event.userInfo.srcUrl)
    context = null;

This will make sure that the context menu will only be shows if context="all" is
requested.

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

Done.

+
+    for (var i = 0; i < contextMenu.length; i++)
+    {
+      if (isContextMenuHidden)
+        return;

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

Why is this check inside the loop? This belongs to the beginning of the
function.

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

Done.

+
+      // Supported contexts are: all, audio, image, video
+      var menuItem = contextMenu[i];
+      if (menuItem.contexts.indexOf("all") == -1 && menuItem.contexts.indexOf(context) == -1)
+        return;

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

This should be continue, not return - there can be multiple menu items.

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

Done.

+      
+      menuItem.item = event.contextMenu.appendContextMenuItem(menuItem.id, menuItem.title);

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

I don't think that menuItem.item is being used, no point setting it.

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

Done.

+    }
+  }, false);
+
+  // Handle context menu item clicks
+  safari.application.addEventListener("command", function(event)
+  {
+    for (var i = 0; i < contextMenu.length; i++)
+    {
+      if (contextMenu[i].id == event.command)
+      {
+        contextMenu[i].onclick(event.userInfo.srcUrl, new Tab(safari.application.activeBrowserWindow.activeTab));
+        break;
+      }
+    }
+  }, false);
+
   // Safari will load the bubble once, and then show it everytime the icon is
   // clicked. While Chrome loads it everytime you click the icon. So in order to
   // force the same behavior in Safari, we are going to reload the page of the