@NekR opened this Issue on August 31st 2014

Here:
https://github.com/piwik/piwik/blob/master/js/piwik.js#L45

You check for JSON2 object in window object which never exists and you always use JS version of JSON instead of builtin.

Also, here https://github.com/piwik/piwik/pull/6066#issuecomment-53999069 man said that you do not use json2.js, but you actually do, or at least json.js which is worse.

Refer to this commend for more details: https://github.com/piwik/piwik/pull/6066#issuecomment-53997506

@mattab commented on August 31st 2014 Member
@NekR commented on August 31st 2014

what is JSON2? any link to standard?

@mattab commented on August 31st 2014 Member

@NekR
This place is our issue tracker, to keep track of bugs and feature requests. Please can you ask questions in the forums: http://forum.piwik.org/

@tsteur commented on August 31st 2014 Member

We could actually indeed use builtin JSON API in case it exists.

JSON2 is a lib that we use as JSON API it is not supported by all browser see http://caniuse.com/#search=json

@halfdan commented on August 31st 2014 Member

@NekR It's a polyfill/fix for broken JSON implementations in some older browsers.

@mattab @NekR has a valid point. Checking for JSON2 on the window object hardly makes any sense as we define JSON2.stringify and JSON2.parse later in the js file.

@NekR commented on August 31st 2014

I knew what json2 is a pollyfill, but almost all browsers have builtin version.

So it's better to have

(function () {
    'use strict';

    if (window.JSON) {
        window.JSON2 = window.JSON;
        return;
    } else {
        window.JSON2 = {};
    }
   // ...

This is fast example how it should be. Or you can just replace everywhere word 'JSON2' to 'JSON'.

@tsteur commented on August 31st 2014 Member

makes sense.

We could even think about removing JSON2 at some point completely. I just had a look and the browsers that do not support JSON builtin are less than 0.5% (IE7 has about 0.2%) and JSON is only needed when custom variables in scope visit are used anyway.

@mattab commented on September 1st 2014 Member

@halfdan you reopened the issue, what do you think should be the scope? maybe Remove JSON2 from piwik.js or so?

@halfdan commented on September 1st 2014 Member

@mattab @NekR has shown an alternative solution that I find reasonable. We can also think about removing JSON2 completely if we decide that those 0.5% that @tsteur mentioned are not worth tracking anymore.

@mattab commented on September 1st 2014 Member

@halfdan could you rename ticket to clarify scope? then feel free to assign to Short term if it's a quick fix

@unwiredbrain commented on October 15th 2014

How about using JSON 3 instead?

JSON 3 [...] is a drop-in replacement for JSON 2. [...] The JSON 3 parser does not use eval or regular expressions. This provides security and performance benefits in obsolete and mobile environments, where the margin is particularly significant.

The functions behave exactly as described in the ECMAScript spec, except for the date serialization [...] it does not define Date#toISOString() or Date#toJSON(). This preserves CommonJS compatibility and avoids polluting native prototypes. Instead, date serialization is performed internally by the stringify() implementation: if a date object does not define a custom toJSON() method, it is serialized as a simplified ISO 8601 date-time string.
-- http://bestiejs.github.io/json3/

Also, this will probably fix #5960.

@tsteur commented on October 15th 2014 Member

Looking good to me. The minified version with 8kb is a bit a lot but should be still ok. Could go with this one for like a year and then support only builtin JSON.

Alternatively now or like in one year we would simply ignore browsers that do not support builtin JSON. If someone still wants those browsers tracked we could let it up to the user to load a separate file before that provides JSON2 or JSON3. So we would check if there is JSON, if not JSON2, then JSON3. If none is there we do not track on this very outdated browsers.

@mattab commented on October 4th 2016 Member

Increasing priority as we probably soon could remove JSON2 from piwik.js

@tsteur commented on October 4th 2016 Member

It's currently pretty much only there for IE7 & IE6 see http://caniuse.com/#search=json

@mattab commented on December 25th 2016 Member

Ok so we won't remove this for a while since we need to support IE7 and IE6. Leaving opened, but decreasing priority

@filipre commented on August 24th 2017

Any updates on this? I guess you are doing this due to contract duties but still encouraging the user to use IE6 sounds extremely insecure.

@mattab commented on September 18th 2017 Member

I guess you are doing this due to contract duties but still encouraging the user to use IE6 sounds extremely insecure.

we don't encourage users to use IE6, but we need to track users on IE6 and IE7 and cannot drop those users from tracking yet (we may in the future, though)

@Findus23 commented on January 30th 2018 Member

Would it be possible to drop the support for IE<7 in Matomo 4 and remove the JSON fallback from the piwik.js?

Now that plugins can modify the piwik.js it may be possible to write a plugin that adds JSON3 for those who still need it (and reducing the load time for everyone else)

Inspired by a forum post

@tsteur commented on January 30th 2018 Member

👍 that is doable I'd say. A plugin that adds support for it may be doable as well.

Just a side note: JSON3 is currently also used for compatibility with some JS frameworks like older versions of prototype and others see eg https://github.com/matomo-org/matomo/issues/11023 .

@robocoder commented on February 12th 2018 Contributor

@NekR: The reason piwik.js called it JSON2 (even though it was using json.js) was because some browsers provided broken window.JSON implementations.

That said, I'm all for dropping the polyfill for Matomo4.

Powered by GitHub Issue Mirror