Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

piwik.js: JSON stringify not working with prototype < 1.7 and mootools < 1.3 #2165

Closed
mattab opened this issue Mar 9, 2011 · 5 comments
Closed
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc. Critical Indicates the severity of an issue is very critical and the issue has a very high priority.
Milestone

Comments

@mattab
Copy link
Member

mattab commented Mar 9, 2011

The bug is with custom variables and strigification.

To reproduce, this is the code:

<html><body><h1>Async code test</h1>

<script type='text/javascript' src="http://demo.magentocommerce.com/js/prototype/prototype.js" ></script>
<p>Great stuff</p>

<!-- Piwik -->
<script type="text/javascript">
var _paq = _paq || [];
(function(){
    var u=(("https:" == document.location.protocol) ? "https://localhost/trunk/" : "http://localhost/trunk/");
    _paq.push(['setSiteId', 1]);
    _paq.push(['setTrackerUrl', u+'piwik.php']);
    _paq.push(['setCustomVariable', '1', 'U','C']);
    _paq.push(['trackPageView']);
    var d=document,
        g=d.createElement('script'),
        s=d.getElementsByTagName('script')[0];
        g.type='text/javascript';
        g.defer=true;
        g.async=true;
        g.src=u+'js/piwik.js';
        s.parentNode.insertBefore(g,s);
})();
</script>
<!-- End Piwik Code -->

The custom vars are then passed as {"1":""C""}

When correct value is: {"1":["U","C"]}

This is known issue and it is working fine when using latest prototype at: https://ajax.googleapis.com/ajax/libs/prototype/1.7.0.0/prototype.js

The patch is:

--- js/piwik.js (revision 4048)
+++ js/piwik.js (working copy)
@@ -120,7 +120,7 @@

         if (value && typeof value === 'object' &&
                 typeof value.toJSON === 'function') {
-            value = value.toJSON(key);
+           // value = value.toJSON(key);
         }

Is it safe to apply in Piwik since we want to be compatible as much as possible, and this case Magento + old prototype represents probably a good number of users.

Is it safe to push this in , or is this code useful?

@robocoder
Copy link
Contributor

It allows an object to self-define how it is to be stringified. It's useful, but not a common use-case, so let's comment it out, and move on.

(Is this the source of the "No data" reports on the forum?)

@mattab
Copy link
Member Author

mattab commented Mar 9, 2011

OK thanks for confirming. It is not fixing other issues than custom variable tracking

@robocoder
Copy link
Contributor

(In [4049]) fixes #2165 - also removed some dead code (previously commented out), and updated comments relating to the webkit bug

@robocoder
Copy link
Contributor

(In [4104]) refs #2165 - tested with mootools 1.2.5 (fails jslint), 1.3.1, and prototype 1.5, 1.6, and 1.7.

  • update to jslint 2011-03-13
  • tweaks to work with jslint's forin:false default
  • more aggressive cookie cleanup

@mattab
Copy link
Member Author

mattab commented Mar 17, 2011

great stuff vipsoft!!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Critical Indicates the severity of an issue is very critical and the issue has a very high priority.
Projects
None yet
Development

No branches or pull requests

2 participants