VelocityJS does not work within the context of Firefox extensions

[seva-luchianov]

Your system information

• VelocityJS version: 2.0.5b
• Browser: Firefox 64.0.2 (64-bit)
• Operating System: Windows 7 Ultimate 64-bit

Checklist

• Is this an issue with code?: Yes
• Is this an issue with documentation?: No
• Have you reproduced this in other browsers?: Yes
• Have you checked for updates?: Yes
• Have you checked for similar open issues?: Yes

Please remember that this is an issue tracker, support requests should be posted on StackOverflow - see CONTRIBUTING.md

Please describe your issue in as much detail as possible:

This is an issue specifically with using velocity inside a Firefox extension.
Regarding the question "Have you reproduced this in other browsers?", I have tested this in chrome 71.0.3578.98 (Official Build) (64-bit) and the issue is not present there, since this is a Firefox specific bug.
Regarding Firefox, I have reported the relevant bug here, it's been linked to an issue open for 3 years now:
https://bugzilla.mozilla.org/show_bug.cgi?id=1523139
However, this can be fixed for Firefox users with a simple patch. In the registerNormalization function, the args are validated by asserting that the constructor is an instance of an Object. If this is changed to window.Object it will fix the Firefox issue.

Steps for reproducing this issue (code):

1. Require VelocityJS in a content script of a Firefox extension
2. VelocityJS will have errors initializing due to a bug in Firefox where (window instanceof Object) evaluates to false for content scripts. (window instanceof window.Object) will evaluate to true in firefox.

JSFiddle example showing issue in action (code):

I cannot make a JS fiddle of this since this issue is only present within the content script of an extension.

[Rycochet]

Will have to think about what to do here (and actually get time for Velocity again) - that change is a complete no-go due to it breaking in several other circumstances (including NodeJS support) - if you can come up with a solution that works in both then feel free to PR something.

[seva-luchianov]

I see. I guess an alternate solution would be to calculate the Object safely ahead of time, like:

const safeObject = function () {
    if (window && window.navigator && window.navigator.userAgent.indexOf("Firefox") > 0 ) {
        return window.Object
    }
    return Object
}()

And then compare against that.

[Rycochet]

It really needs to be dynamic - and that's a lot of extra code to run every tick - can cache things like whether it's Firefox (ugh, I hate browser detection, even if it is the only way to fix browser bugs) - so something along those lines might be valid - and only needed for a specific use-case too, so maybe not much of a hit at all if done right! :-)

[seva-luchianov]

It wouldn't need to run every tick, just once when velocity first loads in. The value would be cached as a global

[seva-luchianov]

Totally agree about browser detection though, it's too bad this issue has been in Firefox for 3+ years with no fix in sight.

[Rycochet]

Sadly the way Velocity works it needs to check if something is a subclass of something else - which means Object - and if someone extends and then replaces Object then caching would break it - if it had a simple test to set a boolean on whether this is needed, then did that const realObject = isBrokenFirefox ? window.Object : Object; that would cover it - I do need to have the time to remind myself how and where (as well as finish updating the tests which is what it's hanging on right now).

[Rycochet]

Making a note here - extensions have got a slightly different context, and different capabilities, than regular browser windows. V3 is getting written to not have an issue with this so it'll work a lot better for these situations - and I really want to ensure that the unit (or possibly integration) testing can actually check for it working in different browser / extensions / nodejs contexts!