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.

Sign up for GitHub

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

Jump to bottom

trackLink does not invoke callback if request is done via GET ? #10153

Closed
er314 opened this issue May 15, 2016 · 5 comments
Closed

trackLink does not invoke callback if request is done via GET ? #10153

er314 opened this issue May 15, 2016 · 5 comments
Labels
Bug For errors / faults / flaws / inconsistencies etc. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement.
Milestone
2.16.2

Comments

@er314
Copy link
Contributor

er314 commented May 15, 2016 •
edited

Trying to understand why it was working with POST, but could not get it to work with GET,
so I looked at the js code, function getImage,
whatever I try, image.src fails onerror.
Then tried playing with request parameters, saw "send_image" was set to 0
-> I set it to 1
-> now it works...
can you reproduce ?

my js tracking code :

  var tracking_callback = function() { console.log('hello callback'); };
  var _paq = _paq || [];
 //_paq.push(['setRequestMethod', 'POST']);
 _paq.push(['trackLink', 'http://127.0.0.1:81/tools/contact/', 'link', null, tracking_callback]);
 (function() {
    var u='//127.0.0.1:81/analytics/';
    _paq.push(['setTrackerUrl', u+'piwik.php']);
    _paq.push(['setSiteId', 1]);
    _paq.push(['setLinkTrackingTimer', 300]);
    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);
 })();
@tsteur tsteur added Bug For errors / faults / flaws / inconsistencies etc. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement. labels May 16, 2016
@tsteur tsteur added this to the 2.16.2 milestone May 16, 2016
@tsteur
Copy link
Member

tsteur commented May 16, 2016

Good point, that's a bug. Makes sense to request an image 👍

@mattab I move it into 2.16.2 as it's easy to fix but feel free to move it out.

@er314 er314 mentioned this issue May 22, 2016
Closed
@mattab
Copy link
Member

mattab commented Jul 12, 2016

@er314 Could you maybe create your PR in the piwik repository, rather than in your fork? Then it makes it possible for us to merge the PR.

Quick review: I think a good way to fix this issue would be to edit getImage method, and replace send_image=0 (if found) by send_image=1.

@mattab mattab modified the milestones: 2.16.x (LTS), 2.16.2 Jul 12, 2016
@er314
Copy link
Contributor Author

er314 commented Jul 12, 2016

Hi,
I did create some PR for some other bugs which I found, and for which I was rather certain about my proposed fix.
Here however, I have zero confidence about the fix, hence no dubious PR proposal... :-)

I have reviewed again the code (piwik.js + core/Tracker/Response.php) , and I think you're right :
getImage() is used exclusively in case of GET, so if we perform the replacement here, then

  • current standard behaviour with default POST requests will be unchanged
  • this does allow GET requests to load the 1x1 image, and to call the callback, thus fixing the issue
    -> I'm doing the PR :-)

@er314 er314 mentioned this issue Jul 12, 2016
Merged
@tsteur
Copy link
Member

tsteur commented Jul 12, 2016

👍

@mattab mattab modified the milestones: 2.16.2, 2.16.x (LTS) Jul 13, 2016
@mattab
Copy link
Member

mattab commented Jul 13, 2016

Kuddos @er314 for the bug report, and the PR!

@mattab mattab closed this as completed Jul 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug For errors / faults / flaws / inconsistencies etc. Help wanted Beginner friendly issues or issues where we'd highly appreciate community's help and involvement.
Projects
None yet
Milestone
2.16.2
Development

No branches or pull requests
3 participants
@tsteur @mattab @er314