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

Unexpected hijacking user after clicking on sparkline and chart #5970

Closed
rivadav opened this issue Aug 11, 2014 · 7 comments
Closed

Unexpected hijacking user after clicking on sparkline and chart #5970

rivadav opened this issue Aug 11, 2014 · 7 comments
Assignees
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Milestone

Comments

@rivadav
Copy link

rivadav commented Aug 11, 2014

Visitors / Overview

When I click on the sparkline in Report and then click on a point in Evolution over period chart, I'm redirected to Dashboard.

mgmhwab8py

@rivadav rivadav closed this as completed Aug 11, 2014
@rivadav rivadav changed the title test Unexpected hijacking user after clicking on a selected day on sparkline Aug 11, 2014
@rivadav rivadav reopened this Aug 11, 2014
@rivadav rivadav changed the title Unexpected hijacking user after clicking on a selected day on sparkline Unexpected hijacking user after clicking on sparkline and chart Aug 11, 2014
@tsteur
Copy link
Member

tsteur commented Aug 18, 2014

Tried to fix it but couldn't without putting a lot of work into a proper solution (probably days, weeks!?!) The problem is kinda in the nature of how Piwik works right now. Could be probably easily fixed once visualizations use AngularJS.

The problem is the following: The first time the evolution is requested this is done with VisitsSummary.index action. Once you click on a sparkline the updated evolution is requested via VisitsSummary.getEvolutionGraph. This action/URL is not registered in the menu and therefore it cannot build the URL to reload the same page here:
https://github.com/piwik/piwik/blob/master/plugins/CoreVisualizations/JqplotDataGenerator/Evolution.php#L169

I thought about workarounds and tried different solutions but the only one that could work would be to always hardcoded replace the module/action in the URL to open with the current module/action from the URL (retrieved via broadcast.getValueFromHash()) here: https://github.com/piwik/piwik/blob/master/plugins/CoreVisualizations/javascripts/jqplotEvolutionGraph.js#L92 . This would mean the evolution would always reload the same page, even if you want a different behavior. This component should not have such knowledge and is rather bad design since you would not expect this (it should exactly open the passed URL and not modify it) but it would be maybe ok if we always want this behavior. Of course we would in this case make the decision also for plugin developers.

@rivadav
Copy link
Author

rivadav commented Aug 19, 2014

@tsteur
Thanks for detailed explanation! It's rather rare case so perhaps we can leave it for now if it's too time consuming.

@mattab
Copy link
Member

mattab commented Aug 20, 2014

I thought about workarounds and tried different solutions but the only one that could work would be to always hardcoded replace the module/action in the URL to open with the current module/action

this sounds like a fine solution to always redirect the user to the currently viewed module/action - @tsteur maybe this is appropriate solution to this issue?

(ie. it's better to redirect always to current module/action rather than redirect to dashboard)

@mattab mattab added this to the Piwik 2.6.0 milestone Aug 20, 2014
@tsteur
Copy link
Member

tsteur commented Aug 20, 2014

Yeah, we just have to be aware that any other behavior won't be possible anymore afterwards as it is a hack. Even for plugin developers who maybe want a different behavior as the widget could be used in many different ways... I think it would be maybe not necessarily a bug if we would always redirect to the dashboard but sometimes this and sometimes an other behavior is a bug. We can add the hack now and remove it once we rewrite the visualizations with Angular JS.

@mattab
Copy link
Member

mattab commented Aug 20, 2014

sometimes this and sometimes an other behavior is a bug.

+1

the question is whether it is better to:

  1. redirect always to dashboard
  2. redirect always to current module/action

I vote for 2) @tsteur does it make sense?

@tsteur
Copy link
Member

tsteur commented Aug 20, 2014

yes

tsteur added a commit that referenced this issue Aug 21, 2014
… in the evolution chart, even if the metric was changed
@tsteur
Copy link
Member

tsteur commented Aug 21, 2014

Should work. Feel free to reopen in case of any issues

@tsteur tsteur closed this as completed Aug 21, 2014
@tsteur tsteur self-assigned this Aug 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

No branches or pull requests

3 participants