@robocoder opened this Issue on February 7th 2011 Contributor

Missing unit tests for:

  • getVisitorId
  • getCustomData
  • setCustomVariable
  • getCustomVariable
  • deleteCustomVariable
  • setCustomUrl
  • setCookieNamePrefix

Cannot test in WebTest or QUnit:

  • setConversionAttributionFirstReferrer
  • setSessionCookieTimeout
  • setCookieDomain
  • setCookiePath
  • setVisitorCookieTimeout
  • setReferralCookieTimeout

Will require Selenium:

  • killFrame
  • redirectFile
  • setHeartBeatTimer

These tests are incomplete:

  • JSON object - Douglas Crockford's github repository doesn't have any tests
  • addListener
  • enableLinkTracking
@mattab commented on February 14th 2011 Member

Attachment: IE8 BUG: outputs JSON.parse error on second refresh
index2.php

@robocoder commented on February 8th 2011 Contributor

(In [3863]) refs #2072 - add some missing unit tests

  • getVisitorId
  • getCustomData
  • setCustomUrl
  • addListener
  • enableLinkTracking
@mattab commented on February 14th 2011 Member

It would be nice to re-enable getVisitorId() but this is not critical for 1.2

@mattab commented on February 14th 2011 Member

In IE, there seem to be an error in JSON.parse when trying to decode the cookie.

The attached file should replicate the issue (ie 8). The error only shows on the second refresh.

@mattab commented on February 14th 2011 Member

Opera also fails (tested on Windows), with this test use case.

@mattab commented on February 14th 2011 Member

(In [3903]) Refs #2072

  • Somehow, the JSON string wouldn't parse properly in IE and Opera.
    However the normal eval() works. Of course, it's not ideal so it would be good to use one of the Parse that would work?
  • Leaving the JSON.parse code in piwik.js for now...
@robocoder commented on February 17th 2011 Contributor

Opera has a built-in/native JSON implementation since Opera 10.50. I've already found a bug in its stringify() implementation (reproduceable with 10.62 and 11.01), so I'll be tweaking piwik.js to always use Crockford's JSON module (even if the browser has native support). This should give us more consistent results across platforms.

@robocoder commented on February 17th 2011 Contributor

(The native JSON implementation was introduced in IE8. Hence the JSON.parse differences.)

@mattab commented on February 17th 2011 Member

I was echoing in the implementation in json.js and it was executed I think. I just put a document.write(err.message); in the piwik catch block and it was showing the exception thrown in JSON.parse

Maybe they interact somehow?

@mattab commented on February 17th 2011 Member

I'm not sure why (this is not a regression apparently), but sometimes downloads eg. http://piwik.org/latest.zip is counted as an outlink in Piwik. For example, yesterday Piwik missed 39 out of 310 downloads or 13% - there might be a bug somewhere in the JS ?

@robocoder commented on February 17th 2011 Contributor

That's the behaviour for subdomains (e.g., de.piwik.org) if you don't use tracker.setDomains('*.piwik.org').

@robocoder commented on February 17th 2011 Contributor

Replying to matt:

I was echoing in the implementation in json.js and it was executed I think. I just put a document.write(err.message); in the piwik catch block and it was showing the exception thrown in JSON.parse

Maybe they interact somehow?

shrug I'm not getting the exception with my dev version of piwik.js on IE8 or Opera.

@mattab commented on February 17th 2011 Member

on trunk, I apply this patch:

### Eclipse Workspace Patch 1.0
#P trunk
Index: js/piwik.js
===================================================================
--- js/piwik.js (revision 3936)
+++ js/piwik.js (working copy)
@@ -1105,7 +1105,9 @@

                if (cookie.length) {
                    //JSON.parse doesn't work in IE and Opera for an unknown reason - see comment in <a href='/2072'>#2072</a>
-                   cookie = eval('('+cookie+')');
+//                 cookie = eval('('+cookie+')');
+                   
+                   cookie = JSON.parse(cookie);
                    if (isObject(cookie)) {
                        return cookie;
                    }

then create a index.php for your root directory containing hte following code:

<html><body>
<!-- Piwik -->
<script type="text/javascript">
var pkBaseURL = (("https:" == document.location.protocol) ? "https://localhost/trunk/" : "http://localhost/trunk/");
document.write(unescape("%3Cscript src='" + pkBaseURL + "js/piwik.js' type='text/javascript'%3E%3C/script%3E"));
</script><script type="text/javascript">
try {
var piwikTracker = Piwik.getTracker(pkBaseURL + "piwik.php", 1);
piwikTracker.setSessionCookieTimeout(10000);
piwikTracker.setCookieNamePrefix('PREFIX');
piwikTracker.setCustomVariable(1, 'visitor type', 'engaged');
piwikTracker.setCustomVariable(2, 'INDEX2.php', navigator.userAgent.substr(30));
piwikTracker.setCustomVariable(6, 'loggedIN', 'yes');

piwikTracker.trackPageView();
piwikTracker.setCustomVariable(3, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa');
piwikTracker.trackPageView();

piwikTracker.enableLinkTracking();

} catch( err ) {
document.write(err.message + "<br/>");
}
</script><noscript><p><img src="http://localhost/trunk/piwik.php?idsite=1" style="border:0" alt="" /></p></noscript>
<!-- End Piwik Tracking Code -->
<a href='http://localhost/index.php'>CODE JS INDEX</a>

with your piwik trunk installed in localhost/trunk

I see the JSON.parse message displayed in IE 8.0.6001.

After upgrading Opera, opera doesn't show an error anymore, nice!

But still IE :(

@robocoder commented on February 18th 2011 Contributor

Ran into another bug in Opera 10.62 and 11.01 where the browser may spontaneously duplicate an HTTP request (i.e., the server will receive two requests for the same URL, including query parameters). This one has no workaround. It's spurious, so I use a loop to increase the chances of reproducing the bug.

Filed as DSK-328881:

<script type="text/javascript">
var myClass = function () {
    this.getImage = function (i) {
        var image = new Image(1, 1);
        image.onLoad = function () { };
        image.src = 'something.php?data=123&q=' + i;
    }
};

for (var i = 0; i < 20; i++) {
    var myObj = new myClass();
    myObj.getImage(i);
}
</script>
@robocoder commented on February 19th 2011 Contributor

in r3939, added more unit tests

@mattab commented on February 19th 2011 Member

excellent work Anthon, I wouldnt have thought of the native JSON conflict. but why was the JSON function triggered?

Is there any manual testing to be done, for example the following functions and check the cookies:
* setConversionAttributionFirstReferrer
* setSessionCookieTimeout
* setCookieDomain
* setCookiePath
* setVisitorCookieTimeout
* setReferralCookieTimeout

Regarding getVisitorId() it looks like it now works when called before track*, excellent. Maybe we should advertise this feature as it could lead to interesting features and integrate (with the Live! API) cool features.

Let me know. Otherwise, please close the ticket as I don't have the error anymore and code & tests looks good

@robocoder commented on February 19th 2011 Contributor

(In [3944]) refs #2072 - handle misconfigured "*." wildcard in setCookieDomain()

@robocoder commented on February 19th 2011 Contributor

(In [3945]) refs #2072 - refactor to simplify isSiteHostName()

@robocoder commented on February 19th 2011 Contributor

re: comment:17 yes these are tested manually. (For example, using Firefox's "View Cookie Information" to examine the cookie domain and path.)

@robocoder commented on February 20th 2011 Contributor

FYI piwik.js unit tests ran successfully on:

  • Internet Explorer 5.5, 6, 7, 8, 9rc
  • Firefox 1.5, 2.0.0, 3.0.5, 3.1b2, 3.5.2, 4.0b7
  • Safari 3.1.2, 3.2.1, 4.0.3, 5.0.2

On Win98SE with IE6, the unit tests passed but ran twice (reported 544 tests). On other version of Windows with IE6, the unit tests ran correctly (reporting 272 tests).

Other browsers included:

  • AOL Explorer 1.5
  • Android Browser 2.2
  • Mobile Safari 4.0
  • Camino 1.66, 2.0.1
  • Netscape 9.0.0.6
  • Opera Mini 5.1
  • Opera 9.27, 9.52, 9.63, 10.63, 11.00
  • Shira 2.2
  • Chrome 5.0, 6, 9.0
  • Seamonkey 1.1.12

The unit tests failed on:

  • Firefox 1.0.8
  • Mozilla 1.7
  • Netscape 4.8, 7.2, 8.1.2
  • Opera 6.0.6, 8.0.2

Untested on:

  • Opera 7.54, 8.54
  • Konqueror 3.5
  • Seamonkey 2.0
@robocoder commented on February 20th 2011 Contributor

For the unit tests that failed, basic tracking code worked for:

  • Firefox 1.0.8
  • Mozilla 1.7
  • Netscape 7.2
  • Netscape 8.1.2
  • Opera 8.0.2

These errored out:

  • Netscape 4.8 - javascript error (illegal character)
  • Opera 6.0.6 - dies on line: throw new Error
@robocoder commented on February 20th 2011 Contributor
  • Opera 7.54 - (JSON cx) regexp compile error
  • Opera 8.54 - basic tracking works
@mattab commented on February 20th 2011 Member

wow very impressive test suite! I think as long as the basic tracking at least works, it is good. Of course it would be nice if at least there was no error thrown, but I think the browsers that fail the tests are definitely very old and not expected.

This Issue was closed on February 20th 2011
Powered by GitHub Issue Mirror