Opened 6 years ago

Closed 5 years ago

Last modified 3 years ago

#355 closed Bug (fixed)

piwik_log() kills the DOM when called: need new clean OO version of the JS tag

Reported by: remy.damour Owned by: vipsoft
Priority: critical Milestone: Piwik 0.4
Component: Core Keywords:
Cc: tecumtah@… Sensitive: no

Description (last modified by matt)

Hi,

First, thanks for Piwik, it look greats, I'm sure it will even get better.
Second, I filled this ticket as bug, because I did not expect piwik.js->piwik_log() to wipe out my entire DOM when called once DOM has been loaded.

Could it be possible to remove any call to document.write or document.writeln from within piwik.js (or make them optional, or put them into generated javascript inserted on each page)?

The reason is the following:

  1. I don't like having javascript within html doc, I prefer to have all my js code in dedicated .js files => I moved js code into outside script

(benefit: code cleaner + most importantly does not interfer with page execution (on my configuration, piwik.js is located on another hostname => my page load freezes up until the script piwik.js is loaded if I use javascript tags, with the consequence of delaying dom load and other js code execution for several seconds).

  1. when you call document.write/document.writeln once your dom has been loaded, it clears all page content => calling piwik_log() once dom is loaded clears all page content

If you could simply remove "document.writeln('<img src="'+_pk_src+'" alt="Piwik" style="border:0" />');" from within piwik.js (or make it optional using a parameter on piwik_log function), it would enhance a lot piwik flexibility (and make my day :-)).

Below is the js code I use (based on mootools1.2) to load piwik.js and call piwik_log from external .js file once all DOM is loaded (maybe it can be useful to other people):

window.addEvent('domready', function() {
	
	function logPiwikData() {
		try {
			var piwik_loaded = !!piwik_log;
		} catch (x) {
			var piwik_loaded = null;
		}
		if (piwik_loaded) {
			piwik_log.apply(this, arguments);
		} else {
			arguments.callee.delay(500, this, arguments); // piwik.js not loaded yet
		}
	}
	
	var pkBaseURL = (("https:" == document.location.protocol) ? "https://piwik.qc4web.com/" : "http://piwik.qc4web.com/");
	var piwik_action_name = document.title;
	var piwik_idsite = 1;
	var piwik_url = pkBaseURL + "piwik.php";
	new Element('script').set('src', pkBaseURL + 'piwik.js').set('type','text/javascript').injectInside(document.body);
	logPiwikData(piwik_action_name, piwik_idsite, piwik_url);
	
});

If you try it without commenting out document.writeln call from piwik.js, all your page content is wiped out. Once it's commented, everything works smoothly.

Regards,
Remy

Attachments (1)

piwik_oop-20081023.tar.gz (211.1 KB) - added by matt 5 years ago.
Remy Damour work on rewriting piwik.js in OO

Download all attachments as: .zip

Change History (39)

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

  • Milestone set to Stable release

You are quite right that this is a bug. One solution could be to create an image in Javascript and append this image to the DOM, avoiding the document.write

Actually we would like to rewrite the whole piwik.ks javascript code to use an object oriented structure. If someone would be ready to help on this story, let me know! :)

comment:2 Changed 6 years ago by remy.damour

Hi Matt,

I've made such implementation. It's quite long to write it all here, I've set it up in my blog: http://qc4blog.com/?p=114

You can use the source code as you want. I hope you will find it usefull.
If you have any question, do not hesitate.

By the way, when generating code to be inserted within all pages, it could be useful to add "?idsite=N" to <noscript> url so that it will get to correct id site.

Regards,
Remy

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

  • Description modified (diff)
  • Milestone changed from Stable release to DigitalVibes
  • Summary changed from piwik_log() wipes out entire DOM when called once dom is loaded to piwik_log() kills the DOM when called: need new clean OO version of the JS tag

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

Note: it obviously needs heavy testing

  • on all majors browsers last versions + popular sub versions
  • needs to be compared to GA and other to check that numbers are still the same

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

See also the change suggested in the piwik-hackers mailing list: http://lists.piwik.org/pipermail/piwik-hackers/2008-August/000337.html

that would make the tracker work in XHTML

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

See also:

Google analytics Tracking API seems really powerful. We wouldn't need so much but the concept is good. http://code.google.com/apis/analytics/docs/gaJSApi.html

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

Also consider to build in "time tracking" to show a report of the speed of the website, enabled via a boolean in the javascript. Does the tracker need to be at top?

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

  • Cc tecumtah@… added

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

  • Keywords javascript js tag api tracking added

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

We should also consider reusing the open source google analytics flash library to track piwik flash events: http://code.google.com/apis/analytics/docs/flashTrackingSetupFlash.html

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

Interesting suggestion from the forums, maybe worth adding to the requirements:
http://forum.piwik.org/index.php?showtopic=57

I am newbie in JavaScript, but is it possible create "google analytics like" javascript code?

I mean: insert only short script tag(s) with link in monitoring pages like this

<script src="http://my_site.com/piwik.js" type="text/javascript"></script>
<script type="text/javascript">some_site_identifier=XXX</script>

comment:15 Changed 5 years ago by vipsoft (robocoder)

As reported on piwik-hacker, piwik.js doesn't recognize <AREA> elements. (Workaround is to use <A>.)

Changed 5 years ago by matt (mattab)

Remy Damour work on rewriting piwik.js in OO

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

microsoft analytics readable code can be found on http://utilitybase.com/paste/11219 (for information only)

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

  • we have to keep backward compatibility by providing old functions as bridges to the new OO class
  • whenever applicable, we should copy google analytics JS API, for simplicity of use and clarity. API doc available on http://code.google.com/apis/analytics/docs/gaJSApi.html. One suggested difference: piwik public methods don't start with _ ; private methods start with _
  • I confirm that we should drop the DOM ready condition, drop usage of global variables
  • you mention #151 but doesn't seem related?
  • existing tracking options (see http://piwik.org/docs/javascript-tag-documentation/ ) should have a simple OO alternative (setDownloadExtensions, setLinkTrackingTimer, disableLinkTracking, etc.). Old way of setting global variables piwik_* should be deprecated. the new code is backward compatible (for example, the init method reads the piwik_* and calls the right newly named methods).
  • Reponse to: 6. Change call from piwik_log(actionName, idSite, piwikURL) to Piwik.init(optionsArray). Rather than using this technique, the final JS code should look simpler that the current one, and more like the GA script, ie.
    [...]
    var pageTracker = piwik.getTracker(piwikSiteId, piwikUrl);
    pageTracker.trackPageview(); // or pageTracker.trackPageview(document.title);
    [...]
    

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

  • as a new requirement, we shall make sure that Piwik doesn't produce javascript error when the piwik.js is blocked by ad-blocking extensions. Currently the code tries to call piwik_log which is not defined and results in a javascript error. [whos.amung.us See forum for more info].

comment:19 Changed 5 years ago by vipsoft (robocoder)

See also ticket #549.

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

  • new requirement: piwik.js would provide a setDocumentTitle(). This method would set the "title" http parameter to piwik.php to the custom value. This value would then be used to build best pages by title, see #530
  • Piwik currently build the "actions" report based on URL, or based on the custom piwik_action_name = ''; value if specified. If not, the report is built based on the URL. This existing piwik_action_name = ''; would logically be replaced by a piwik.setUrl(). The parameter to piwik.php would be renamed from action_name to custom_url or similar.

This is open to suggestions..

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

  • Priority changed from normal to critical

comment:23 Changed 5 years ago by vipsoft (robocoder)

  • Owner set to vipsoft
  • Status changed from new to assigned

comment:24 follow-up: Changed 5 years ago by doodlez

has anyone figured out a way around this yet. I am attempting to implement tracking from within a swf file and the only thing keeping this from completion is that the DOM blanks out after the piwik_log is called

comment:25 Changed 5 years ago by remy.damour

@doodlez, you might find this useful: http://www.qc4blog.com/?p=114
oop js file (not using document.write):

http://qc4blog.com/wp-content/uploads/2008/09/piwik_oop-029.js

Regards, Remy

comment:26 in reply to: ↑ 24 ; follow-up: Changed 5 years ago by remy.damour

Replying to doodlez:

has anyone figured out a way around this yet. I am attempting to implement tracking from within a swf file and the only thing keeping this from completion is that the DOM blanks out after the piwik_log is called

@doodlez, you might find this useful: http://www.qc4blog.com/?p=114 oop js file (not using document.write):

http://qc4blog.com/wp-content/uploads/2008/09/piwik_oop-029.js

Regards, Remy
(oups, missed reply button)

comment:27 in reply to: ↑ 26 Changed 5 years ago by doodlez

@doodlez, you might find this useful: http://www.qc4blog.com/?p=114 oop js file (not using document.write):

http://qc4blog.com/wp-content/uploads/2008/09/piwik_oop-029.js

Regards, Remy
(oups, missed reply button)

I attempted to integrate your code, but it didn't appear to be tracking.

comment:28 follow-up: Changed 5 years ago by doodlez

this is what I attempted to implement:

<script language="javascript" src="http://www.****.com/piwik/piwik_oop-0.2.9-min.js" type="text/javascript"></script>

<script type="text/javascript">

function setActionName(page){
piwik_action_name = page;
piwik_idsite = 1;
piwik_url = "http://www.****.com/piwik/piwik.php";
piwik_log(piwik_action_name, piwik_idsite, piwik_url);
}

</script><object>

<noscript><p>Web analytics <img src="http://www.****.com/piwik/piwik.php?idsite=1" style="border:0" alt="piwik"/></p>
</noscript></object>

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

  • Milestone changed from 2- DigitalVibes to 1 - Piwik 0.4

this will be the main new feature in Piwik 0.4 release.

comment:30 Changed 5 years ago by vipsoft (robocoder)

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

(In [1151]) fixes #355 - OO version of piwik.js which no longer modifies DOM;
remove misc/testJavascripTracker (interactive tests);
add tests/javascript (QUnit unit tests);
fixes #661 - use click event instead of mousedown;
fixes #549 - define your own download/outlink tracking classes;
fixes #82 - add hook interface for module

comment:31 Changed 5 years ago by vipsoft (robocoder)

(In [1157]) refs #355 - minor changes per review

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

(In [1177]) - refs #355 now displaying the new OO JS tag by default in install & in admin UI + showing some help + renaming the "js code" to consistent "JavaScript Tracking Tag"

comment:33 Changed 5 years ago by vipsoft (robocoder)

(In [1185]) refs #355 - accommodate malformed URIs that contain an illegal space
after the pseudo-protocol

comment:34 Changed 5 years ago by goodware

comment:35 in reply to: ↑ 28 Changed 5 years ago by albass

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

  • Keywords document.write javascript js tag api tracking document.writeln removed
  • Sensitive unset

comment:37 Changed 3 years ago by phillipnoyce

comment:38 Changed 3 years ago by phillipnoyce

Note: See TracTickets for help on using tickets.