It doesn't work when you try to open a URL: Since 'referrer' has block scope, it is always undefined at line 693, so an exception is thrown and we end up in the 'catch' without opening anything: https://dxr.mozilla.org/comm-central/rev/4f8486123b817d6b18bf2684a0f7e5facf725ce6/mail/base/content/mailWindow.js#693

Here's the obvious fix. Stand-by for a debug patch to see it working ;-)

This is misusing MailSetCharacterSet() as a test function. Set the charset of a message to Unicode and BMO will open in a new tab. Set any other charset and Google will open. This exercises two distinct cases internal cases. Note that openURI() is also called when going to Get Add-ons, clicking on any add-on that's suggested, for example "Mail Merge", and then clicking onto a link, for example this one: Website https://addons.mozilla.org/addon/mail-merge/

Comment on attachment 8815353 [details] [diff] [review] 1321013-fix-openURI.patch (v1) Review of attachment 8815353 [details] [diff] [review]: ----------------------------------------------------------------- Where do we get a proper JS compiler that catches this in advance? :) Nice find. Looks like this was there even at the time of last change of that line in 2012.

C-C (TB 53): https://hg.mozilla.org/comm-central/rev/5bb2d288ae3ed011048a8a405eadad245c2448be C-A (TB 52): https://hg.mozilla.org/releases/comm-aurora/rev/5edaddc025cbcf250be0ca07342d85879d9fa0ba

Any reason this was pushed DONTBUILD? I think we should be building normally even on minor changes, in order to catch potential m-c issues earlier. While it is nice to save some build time, I don't think we should be doing this on normal pushes.

I have adopted this scheme: If the push is minor and I'm sure it won't be covered by any test, I push with DONTBUILD unless between the last full build and my push there was an M-C merge. Take a few cases: This case: Pushed Sat Dec 3, 20:10:33 Previous full build: Sat Dec 3, 18:56:47 Last M-C merge: Sat Dec 03 06:17:44 Decision: DONTBUILD A previous case: Pushed Sat Dec 3, 8:15:22 Previous full build: Sat Dec 3, 0:38:38 Last M-C merge: Sat Dec 03 06:17:44 Decision: BUILD despite trivial change (one JS line) Believe me, I'm the first one who wants to see new bustage ;-) But it doesn't make sense to build the exact same binaries twice and run the exact same tests. There are other points to consider: These days we have a daily block list update on all repositories, so anything you push will be picked up within 24 hours. Before that wasn't the case, there the Dailies were built on the previous non-DONTBUILD pushes for days. So I'm more inclined to push to C-A and C-B with DONTBUILD than before.