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

Show warning when paq.push is not initialized correctly #10889

Merged
merged 7 commits into from Dec 13, 2016

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Nov 21, 2016

refs #10878

This is PR for Piwik 3 for #10888

To test use eg

<html>
<head></head>
<body>
<script type="text/javascript" src="//apache.piwik/piwik.js"></script>
</body>
</html>

Then initialize tracker after document was loaded via _paq.push. You will see a console message and no created tracker. When using _paq.push for tracking, the Tracker needs to be configured upfront. Otherwise bugs may occur.

@tsteur tsteur added Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Nov 21, 2016
@tsteur tsteur added this to the 3.0.0-b4 milestone Nov 21, 2016
@tsteur
Copy link
Member Author

tsteur commented Nov 21, 2016

Updated PR to still track but also log a message

@tsteur
Copy link
Member Author

tsteur commented Nov 21, 2016

Updated PR again to not track when tracker is not initialized upfront

@mattab
Copy link
Member

mattab commented Dec 1, 2016

  • Ideally when the user calls setTrackerUrl and setSiteId even after the tracker was initialised, the tracker should initialise without error. Ie. the following code should not trigger an error but currently it does:
<!-- Piwik -->
<script type="text/javascript">
  var u="//localhost/piwik-master/";

  (function() {
    var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0];
    g.type='text/javascript'; g.async=true; g.defer=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s);
  })();

  var _paq = _paq || [];

setTimeout(function() {
  _paq.push(['setTrackerUrl', u+'piwik.php']);
  _paq.push(['setSiteId', '1']);
  _paq.push(["setCustomVariable", 1, "", "", "visit"]);
  _paq.push(['trackPageView']);
  _paq.push(['enableLinkTracking']);
}, 1000);
</script>
<!-- End Piwik Code -->
  • If we don't init tracker when setSiteId/setTrackerUrl were called in the right order (even after tracker init), then we should track the data for sure, or alternatively not merge PR for now

  • Suggested message: Piwik Analytics error: _paq.push() was used before the Piwik tracker was correctly initialized. To initialise the tracker please make sure to first call the two methods 'setTrackerUrl' and 'setSiteId' via _paq.push(). Alternatively, if you do not use _paq.push you may intialise the Piwik tracker via: 'Piwik.getTracker( trackerUrl, siteId )' and call methods on the object returned by getTracker.

  • you wrote in the message: but it may fully work as tracker methods may not be executed in the correct order. for the Piwik.addTracker() method -> do you maybe have an example?

@mattab mattab modified the milestones: 3.0.0-b5, 3.0.0-b4 Dec 1, 2016
@tsteur
Copy link
Member Author

tsteur commented Dec 1, 2016

There are plenty of reasons why it won't fully work just search for issues in the issue tracker that are related to order. Eg we create cookies even though we shouldn't , or create cookies wrong domain etc.

I can't do anything else here. Happy to close PR and it just won't keep working for users. Here was suggestion on top for #10797 (comment) but I likely won't find any time to work on it

@tsteur
Copy link
Member Author

tsteur commented Dec 1, 2016

Also see comments where we discussed it. Eg custom tracker plugins won't be working with it etc

@mattab mattab modified the milestones: 3.0.0, 3.0.0-b5 Dec 5, 2016
@mattab mattab modified the milestones: 3.0.0-rc, 3.0.0 Dec 13, 2016
@mattab mattab merged commit 6e8730e into 3.x-dev Dec 13, 2016
@mattab mattab deleted the latetrackerinit3x branch December 13, 2016 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants