Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#2165 closed Bug (fixed)

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

Reported by: matt Owned by: vipsoft
Priority: critical Milestone: Piwik 1.2.1
Component: Core Keywords:
Cc: Sensitive: no


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="" ></script>
<p>Great stuff</p>

<!-- Piwik -->
<script type="text/javascript">
var _paq = _paq || [];
    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']);
    var d=document,
<!-- End Piwik Code -->

The custom vars are then passed as {"1":"[\"U\", \"C\"]"}

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

This is known issue and it is working fine when using latest prototype at:

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?

Change History (6)

comment:1 Changed 3 years ago by vipsoft (robocoder)

  • Owner set to vipsoft
  • Summary changed from JSON2 not working with prototype packaged in Magento to piwik.js: JSON stringify not working with prototype < 1.7 and mootools < 1.3

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?)

comment:2 Changed 3 years ago by matt (mattab)

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

comment:3 Changed 3 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:4 Changed 3 years ago by vipsoft (robocoder)

(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

comment:5 Changed 3 years ago by matt (mattab)

  • Priority changed from normal to critical

comment:6 Changed 3 years ago by matt (mattab)

great stuff vipsoft!!

Note: See TracTickets for help on using tickets.