andrewplummer / Sugar

A Javascript library for working with native objects.
https://sugarjs.com/
MIT License
4.53k stars 306 forks source link

Current SugarJS incompatible with current Thunderbird (probably also Firefox?) trunk #627

Open jikamens opened 6 years ago

jikamens commented 6 years ago

Attempting to use SugarJS with the 2018-03-15 Thunderbird Trunk build, I get this:

08:37:46.183 TypeError: globalContext[name] is undefined 1 sugar.min.js:791:9
    mapNativeToChainable resource://sendlater3/sugar.min.js:791:9
    createNamespace resource://sendlater3/sugar.min.js:521:5
    setupGlobal/< resource://sendlater3/sugar.min.js:91:7
    forEachProperty resource://sendlater3/sugar.min.js:839:11
    setupGlobal resource://sendlater3/sugar.min.js:90:5
    <anonymous> resource://sendlater3/sugar.min.js:874:3
    <anonymous> resource://sendlater3/sugar.min.js:9:2
    <anonymous> resource://sendlater3/dateparse.jsm:9:1
    <anonymous> chrome://sendlater3/content/composing.js:1:1

I imagine this is also going to impact Firefox since Thunderbird and Firefox share a JavaScript engine.

The trunk build from three days ago does not have this problem. The build transitioned from Thunderbird 60 to Thunderbird 61 in the interim, so I imagine this problem was introduced by a change that was held out of the trunk until the bump from 60 to 61.

jikamens commented 6 years ago

OK, so I've figured out a workaround for this. do with it what you will:

diff --git a/chrome/resource/sugar.js b/chrome/resource/sugar.js
index cf08ea2..273d561 100644
--- a/chrome/resource/sugar.js
+++ b/chrome/resource/sugar.js
@@ -38,6 +38,17 @@
   // The global context. Rhino uses a different "global" keyword so
   // do an extra check to be sure that it's actually the global context.
   var globalContext = typeof global !== 'undefined' && global.Object === Object ? global : this;
+  // `this` in Thunderbird 61+ (and probably upcoming Firefox releases as well,
+  // since TB and FF use the same JavaScript interpreter) is a
+  // NonSyntacticVariablesObject with only a few variables defined in it, not a
+  // BackstagePass with all the global variables including types defined in it.
+  // To detect this, we check if 'Array' is defined, and if not, we fix it
+  // using the method described in https://stackoverflow.com/a/3277192/5090662.
+  // I think that method would actually work all the time everywhere, not just
+  // with TB 61+, but I can't be certain enough about that to just throw away
+  // the logic above. Perhaps Andrew Plummer can, but I can't. ;-)
+  if (typeof globalContext['Array'] === 'undefined')
+    globalContext = Function('return this')();

   // Is the environment node?
   var hasExports = typeof module !== 'undefined' && module.exports;
jikamens commented 6 years ago

See also https://groups.google.com/forum/#!topic/firefox-dev/J2frGYCb7Rg. It looks like this is coming down the pike for Firefox too, not just Thunderbird.

joeskeen commented 6 years ago

I also had this problem when using Angular CLI (Webpack) and trying to use Sugar (in Chrome). @jikamens patch fixed it for me as well. Thanks! Please use this patch in future versions of Sugar!

andrewplummer commented 6 years ago

@jikamens Thank you for this fix and very sorry for the delay here. I believe the fix in #632 should work here as well, although it's strange that they're occurring at similar times as that one should be related to webpack. In any case it's time to overall tighten down the assumptions I'm making about the environment Sugar is running in so I think for now referencing global and window directly with safety checks is the best way. The Function() idea is clever, but I believe that servers with CSP protection will block this, which is something that has come up in the past.

andrewplummer commented 6 years ago

@jikamens Also mind sharing how you're testing Thunderbird?

jikamens commented 6 years ago

@jikamens Also mind sharing how you're testing Thunderbird?

"Testing"? ;-)

My Thunderbird add-on (Send Later) wasn't working properly without my fix. It's working with it. That's the extent of my testing.

jikamens commented 5 years ago

I believe the fix in #632 should work here as well,

If you mean the fix in https://github.com/andrewplummer/Sugar/commit/0cd73d1fc83e16a7b51e02c5b19b9db18d7ac180, nope, it's not working. When I build a custom version of SugarJS from the Git repo using gulp and try to load it into current Thunderbird, the interpreter complains globalContext is null. The line it's complaining about is Sugar = globalContext[SUGAR_GLOBAL]; at the beginning of setupGlobal, so it would seem that neither global nor window is defined when a JavaScript module is loaded with ChromeUtils.import in current Thunderbird.