@tsteur opened this Pull Request on August 6th 2015 Owner

fixes #7926

This is a simpler fix for #7926 compared to #8022 . I tried to understand what the actual problem is and how to solve it for 2 hours but couldn't figure it out even though I debugged this step 30 times. The setting of language and translation etc is very confusing, maybe needs a refactoring at some point similar to how it used to be. There seems to be a cookie reset when creating the config file which seems to cause it but it should still work.

It's not really good that we have to init the language that often. As mentioned we might want to refactor it at some point.

@mnapoli commented on August 6th 2015 Member

The setting of language and translation etc is very confusing, maybe needs a refactoring at some point similar to how it used to be.

I'm not sure I understand what you mean by that: do you want to revert the refactoring that was done to go back to what it was before? FYI the language initialization at the app level is messy because of how Piwik is bootstrapped. The translation component itself is very straightforward and is very similar to Symfony's Translation component (which couldn't be used because of some Piwik specifics).

@tsteur commented on August 7th 2015 Owner

I meant the bootstrapping part of the translation (not the component itself), meaning having to initialize the language 4 times would be worth having a look some day but not needed soonish.

@mnapoli commented on August 7th 2015 Member

Yeah I don't understand either the whole translation init, this should indeed be solved at some point.

@PatchRanger commented on August 7th 2015

That's really simpler.

There seems to be a cookie reset when creating the config file which seems to cause it but it should still work.

I agree, it looks strange that it breaks language, it indeed should still work. My guess is that cookie is not being reset but there appears another cookie with the same name without removing the previous. So at that moment we have 2 cookies with different values. I didn't check it though, it's just my guess.
Anyway, the suggested workaround works and I agree that it would be better to incorporate this pull-request instead of #8022 as it aims the root of the issue not touching unrelated stuff.

This Pull Request was closed on August 13th 2015
Powered by GitHub Issue Mirror