added hook function to appIntegration to handle function-overwrites from other extensions

lib/appIntegration.js

 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-// Extensions like Tab Mix Plus will try to recompile our handlers and fail
-// badly - they manage to replace our handler but the recompiled version
-// won't work because the closure is missing its variables. We replace
-// toString() function to prevent this kind of recompiling on earlier stages
-// (produce a syntax error but still leave the source code viewable).
-function doNotRecompile()
+function hook(obj, name, func)

[Wladimir Palant, 2012/09/26 09:27:50]

Any reason why you've put it here? I expected it to be in a separate module -
this is generic functionality that is not application-specific.

 {
-  let result = Function.prototype.toString.apply(this);
-  return result + "\n$%&!/DO_NOT_RECOMPILE"
+  var orig = obj[name];
+  var origGet = obj.__lookupGetter__(name);
+  var origSet = obj.__lookupSetter__(name);
+  var dumbOverrideAttempt = false;

[Wladimir Palant, 2012/09/26 09:27:50]

You should use let instead of var in Firefox code - the scope of let variables
is defined in a much cleaner way, unexpected scope issues are unlikely here.

(Note that I fixed that issue in one of my changes already)

+  
+  var newFunc = function()
+  {
+    try
+    {
+      func.apply(this, arguments);
+    }
+    catch(e)
+    {
+      Cu.reportError(e);
+    }
+    orig.apply(this, arguments);
+  }
+  newFunc.toString = function()
+  {
+    dumbOverrideAttempt = true;
+    return orig.toString();
+  }
+  
+  obj.__defineGetter__(name, function()
+    {
+      dumbOverrideAttempt = false;
+      return newFunc;
+    }
+  );
+  
+  obj.__defineSetter__(name, function(value)
+    {
+      if (dumbOverrideAttempt)
+      {
+        orig = value;
+      }
+      else
+      {
+        delete obj[name];
+        obj[name] = value;
+      }
+    }
+  );
+  
+  return function() {
+    delete obj[name];
+    obj[name] = orig;
+    if (origGet) {
+      obj.__defineGetter__(name, origGet);
+    }
+    if (origSet) {
+      obj.__defineSetter__(name, origSet);
+    }
+  }

[Wladimir Palant, 2012/09/26 09:27:50]

Inconsistent style here (brackets) - plus there is a missing semicolon at the
return statement (only mentioning because missing semicolons after function
literals seems to be a recurring issue in your code).

(Note that I fixed this issue in one of my changes already)

 }
 
+let wm = new WeakMap();
+let {Prefs} = require("prefs");

[Wladimir Palant, 2012/09/26 09:27:50]

I would be very worried if our generic app-support code were suddenly dependent
on preferences... This seems unused however, remove?

 let {application} = require("info");
 switch (application)
 {
   case "firefox":
   {
     // Firefox
     exports.isKnownWindow = function(window) window.document.documentElement.getAttribute("windowtype") == "navigator:browser";
 
     exports.getURLBar = function(window) "gURLBar" in window ? window.gURLBar : null;
 
     exports.getBrowser = function(window) "gBrowser" in window ? window.gBrowser : null;
 
     exports.applyToWindow = function(window, corrector)
     {
       let urlbar = exports.getURLBar(window);
       if (urlbar && urlbar.handleCommand && !("urlfixerOldHandler" in urlbar.handleCommand))
       {
         // Handle new URLs being entered
-        let oldHandler = urlbar.handleCommand;
-        urlbar.handleCommand = function()
+        let hooks = [];

[Wladimir Palant, 2012/09/26 09:27:50]

From the look of it, we never hook more than one function (in Fennec native we
hook additional functions only temporarily - that's different). How about
putting a single unhook function into the map instead of an array?

+        if (wm.get(window))

[Wladimir Palant, 2012/09/26 09:27:50]

The window should definitely be unknown here - if we are called twice for the
same window then we have bigger problems than having to merge hooks. Instead of
the check here you probably want to replace the outdated !("urlfixerOldHandler"
in urlbar.handleCommand) check above by !wm.has(window).

         {
-          try
+          hooks = wm.get(window);
+        }
+        hooks.push(hook(urlbar, "handleCommand", function()
           {
-            let correction = corrector(window, urlbar.value);
-            if (correction)
-              urlbar.value = correction;
+            try
+            {
+              let correction = corrector(window, urlbar.value);
+              if (correction)
+                urlbar.value = correction;
+            }
+            catch(e)
+            {
+              Cu.reportError(e);
+            }

[Wladimir Palant, 2012/09/26 09:27:50]

The try..catch block here is no longer necessary - the hook() function already
takes care of that.

           }
-          catch(e)
-          {
-            if (e == Cr.NS_BINDING_ABORTED)
-              return;
-            else
-              Cu.reportError(e);
-          }
-          oldHandler.apply(this, arguments);
-        }
-        urlbar.handleCommand.urlfixerOldHandler = oldHandler;
-        urlbar.handleCommand.toString = doNotRecompile;
+        ));
+        wm.set(window, hooks);
       }
     };
 
     exports.removeFromWindow = function(window)
     {
       let urlbar = exports.getURLBar(window);
-      if (urlbar && urlbar.handleCommand && "urlfixerOldHandler" in urlbar.handleCommand)
-        urlbar.handleCommand = urlbar.handleCommand.urlfixerOldHandler;
+      if (urlbar && urlbar.handleCommand && wm.has(window))
+      {
+        let hooks = wm.get(window);
+        for (let i = 0, len = hooks.length; i < len; i++)
+        {
+          hooks[i]();
+        }
+        wm.delete(window);
+      }
     };
     
     exports.openInfobar = function(window, id, message, buttons, persistence)
     {
       let browser = exports.getBrowser(window);
       let infobar = browser.getNotificationBox();
       let notif = infobar.getNotificationWithValue(id);
       
       if (notif)
       {
@@ skipping 22 matching lines @@
     // SeaMonkey
     exports.isKnownWindow = function(window) window.document.documentElement.getAttribute("windowtype") == "navigator:browser";
 
     exports.getURLBar = function(window) "gURLBar" in window ? window.gURLBar : null;
 
     exports.getBrowser = function(window) "gBrowser" in window ? window.gBrowser : null;
 
     exports.applyToWindow = function(window, corrector)
     {
       let urlbar = exports.getURLBar(window);
-      if (urlbar && window.handleURLBarCommand && !("urlfixerOldHandler" in window.handleURLBarCommand))
+      if (urlbar && urlbar.handleCommand && !("urlfixerOldHandler" in urlbar.handleCommand))

[Wladimir Palant, 2012/09/26 09:27:50]

Replacing SeaMonkey code by Firefox-specific code isn't going to work. The
function to hook is window.handleURLBarCommand in SeaMonkey, please restore the
functionality here.

       {
         // Handle new URLs being entered
-        let oldHandler = window.handleURLBarCommand;
-        window.handleURLBarCommand = function()
+        let hooks = [];
+        if (wm.get(window))
         {
-          try
+          hooks = wm.get(window);
+        }
+        hooks.push(hook(urlbar, "handleCommand", function()
           {
-            let correction = corrector(window, urlbar.value);
-            if (correction)
-              urlbar.value = correction;
+            try
+            {
+              let correction = corrector(window, urlbar.value);
+              if (correction)
+                urlbar.value = correction;
+            }
+            catch(e)
+            {
+              Cu.reportError(e);
+            }
+            oldHandler.apply(this, arguments);
           }
-          catch(e)
-          {
-            if (e == Cr.NS_BINDING_ABORTED)
-              return;
-            else
-              Cu.reportError(e);
-          }
-          oldHandler.apply(this, arguments);
-        }
-        window.handleURLBarCommand.urlfixerOldHandler = oldHandler;
-        window.handleURLBarCommand.toString = doNotRecompile;
+        ));
+        wm.set(window, hooks);
       }
     };
-
+    

[Wladimir Palant, 2012/09/26 09:27:50]

I suggest the following setting for the Sublime Text editor:

  "trim_trailing_white_space_on_save": true

     exports.removeFromWindow = function(window)
     {
-      if (window.handleURLBarCommand && "urlfixerOldHandler" in window.handleURLBarCommand)
-        window.handleURLBarCommand = window.handleURLBarCommand.urlfixerOldHandler;
+      let urlbar = exports.getURLBar(window);
+      if (urlbar && urlbar.handleCommand && wm.has(window))
+      {
+        let hooks = wm.get(window);
+        for (let i = 0, len = hooks.length; i < len; i++)
+        {
+          hooks[i]();
+        }
+        wm.delete(window);
+      }
     };
     
     exports.openInfobar = function(window, id, message, buttons, persistence)
     {
       let browser = exports.getBrowser(window);
       let infobar = browser.getNotificationBox();
       let notif = infobar.getNotificationWithValue(id);
       
       if (notif)
       {
@@ skipping 36 matching lines @@
         {
           url = url || this._edit.value;
           try
           {
             let correction = corrector(window, url);
             if (correction)
               url = correction;
           }
           catch(e)
           {
-            if (e == Cr.NS_BINDING_ABORTED)
-              return;
-            else
-              Cu.reportError(e);
+            Cu.reportError(e);
           }
           oldHandler.call(this, url);
         }
         window.BrowserUI.goToURI.urlfixerOldHandler = oldHandler;
         window.BrowserUI.goToURI.toString = doNotRecompile;
       }
     };
 
     exports.removeFromWindow = function(window)
     {
@@ skipping 32 matching lines @@
         {
           let params = Array.prototype.slice.apply(arguments);
           try
           {
             let correction = corrector(window, params[0]);
             if (correction)
               params[0] = correction;
           }
           catch(e)
           {
-            if (e == Cr.NS_BINDING_ABORTED)
-              return null;
-            else
-              Cu.reportError(e);
+            Cu.reportError(e);
           }
           return oldFunc.apply(this, params);
         };
 
         window.BrowserApp.observe = function(subject, topic, data)
         {
           // Huge hack: we replace addTab/loadURI when the observer is
           // triggered. This seems to be the only way to know that the calls
           // originate from user input.
           let method = null;
@@ skipping 49 matching lines @@
     };
 
     break;
   }
   default:
   {
     exports.isKnownWindow = function(window) false;
     break;
   }
 }