Sandcastle-Builder icon indicating copy to clipboard operation
Sandcastle-Builder copied to clipboard

Non-functional without Google Analytics on Firefox 44

Open Zombie-Feynman opened this issue 9 years ago • 8 comments

On Firefox 44, if the Google Analytics script cannot be loaded (e.g. because it is blocked with NoScript, as is my case), Sandcastle Builder is completely non-functional due to the following error:

TypeError: _gaq.push is not a function
    _gaq && _gaq.push(['_trackEvent', 'Load', 'Begin']);

A quick inspection in Firebug reveals that _gaq is of type

Object { __noSuchMethod__=function(),  _link=function(),  _linkByPost=function(),  more...}

which evaluates to true when used in a conditional:

_gaq && true || false;
true

To fix this, just replace _gaq && _gaq.push(...); with

typeof _gaq.push !== 'undefined' && _gaq.push(...);

wherever it occurs.

Zombie-Feynman avatar Feb 25 '16 13:02 Zombie-Feynman

Alternatively, we could just provide a default _gaq={push:function(){}} before the script tries to load (or something of that sort).

pickten avatar Feb 25 '16 20:02 pickten

Alternatively, we could just provide a default _gaq={push:function(){}} before the script tries to load (or something of that sort).

I kinda assumed that it did something like this, but I guess not on firefox.

eternaldensity avatar Feb 25 '16 20:02 eternaldensity

The strange thing is that it works fine with Firefox 43 and earlier. In fact, it may just as well be a Firefox bug, but I don't know enough about the JavaScript standard to tell.

Zombie-Feynman avatar Feb 25 '16 22:02 Zombie-Feynman

Indeed the code already has a workaround for load errors like noscript in castle.js: var _gaq = _gaq || []; That is the code google offer to avoid errors like this. But it seems that _gaq exists and it is not an array nor has a push function so it seems more like an NoScript error loading an alternative script to analytics that do not behave correctly.

Could you open the console and call typeof _gaq;? Just to check at least whats the type of your _gaq objec

friscoMad avatar Feb 27 '16 19:02 friscoMad

Judging from all this, I think it's either null or a string -- either of which is easy to test for. A one liner: if(typeof _gaq != typeof [] || !_gaq.push){_gaq={push:funtion(){}}}

(not [] because that's a bit slower since push would do actual work)

pickten avatar Feb 27 '16 20:02 pickten

typeof _gaq reports "object". Also, typeof [] === typeof _gaq evaluates to true.

Zombie-Feynman avatar Mar 01 '16 08:03 Zombie-Feynman

I'm too lazy to remember what typeof returns, hence the typeof [], but in any case, that essentially means the if catches and prevents this bug, since the issue was that _gaq.push is not a function (i.e. is undefined), and therefore falsy. I'll send a PR shortly, in that case, which should fix this and prevent other bad values from causing problems.

pickten avatar Mar 01 '16 14:03 pickten

Redundacopypasta from my response to the pull request:

Nope, still the same error. It looks like the updated code somehow isn't being called before window.onload during normal execution. This causes the call to _gaq.push in Molpy.DefinePersist to trigger the error.

Oddly enough, setting breakpoints and stepping through the execution does appear to lead to _gaq being defined before Molpy.DefinePersist is called.

Zombie-Feynman avatar Mar 01 '16 16:03 Zombie-Feynman