matomo-org / matomo

Empowering People Ethically with the leading open source alternative to Google Analytics that gives you full control over your data. Matomo lets you easily collect data from websites & apps and visualise this data and extract insights. Privacy is built-in. Liberating Web Analytics. Star us on Github? +1. And we love Pull Requests!
https://matomo.org/
GNU General Public License v3.0
19.87k stars 2.65k forks source link

Bug in Swedish translation when displaying negative or zero values #18802

Closed melker closed 2 years ago

melker commented 2 years ago

Posting two screenshots:

Displayed in English (Correct) matomo-bug-en

Displayed in Swedish (with some highlighted areas by me) matomo-bug-sv

Bugs in Swedish version:

sgiehl commented 2 years ago

Hi @melker Thanks for creating the issue. I was able to reproduce it locally. Guess that is due to the fact, that in Swedish it doesn't use a "english" minus sign and such can't detect that the number is lower 0. Guess this might happen for other languages as well.

tsteur commented 2 years ago

@sgiehl do you know how easy it is to fix this one?

sgiehl commented 2 years ago

Maybe it's enough to fetch unformatted data for the sparklines and format them only for the display values. Or have unformatted data as metadata like we have for totals. Would need to have a look at that to say more

melker commented 2 years ago

I'd say the main problem is the weird translation. It should be the same signs as in the English version, at least in Swedish. (Other languages may have other signs.)

sgiehl commented 2 years ago

@melker we are using the "official" data from Unicode CLDR: See https://raw.githubusercontent.com/unicode-org/cldr-json/39.0.0/cldr-json/cldr-numbers-full/main/sv/numbers.json

melker commented 2 years ago

@sgiehl not really....

It says: "minusSign": "−",

and on the page (my screenshot) you have: "+-" (both plus and munis)

(reservation if I enter the wrong dash-sybol)

sgiehl commented 2 years ago

@melker That minus sign differs from the english one. Even though it's not really good visible in some fonts. In my browser a can see a small difference in the length of the signs below:

"minusSign": "−",
"minusSign": "-",

And the reason that you see a plus and minus sign is something that happens in the code. Normally numbers don't have a plus sign in front. So we are adding one if the number is not negative. But the check for negative numbers fails in that case, as it already uses the formatted number, which uses another minus sign...

melker commented 2 years ago

That look bit weird to my the the English uses the hyphen-minus and not the formal minus character while the Swedish version uses the formal minus character... well I and not a person really into those things... but still It seems weird to me. Now moving forward instead...

So you need to make the pos/neg check locale aware.

How about the zeros? How come they get the same treatment as negative values? (I don't see what is used on the Unicode CLDR)

@sgiehl

sgiehl commented 2 years ago

So you need to make the pos/neg check locale aware.

Exactly. For the zero value that is a similar problem. The formatted percent value contains a non breaking space, which might also break the numeric detection. As noted somewhere above, we need to do such comparisons with unformatted numbers to fix all those issues.

tsteur commented 2 years ago

@sgiehl if there is a different sign, does that mean we can fix that one easily? At least for some languages by putting in the translated minusSign to detect negative values?

sgiehl commented 2 years ago

@tsteur yes that might work. Even though it would be more a workaround than a proper fix.

sgiehl commented 2 years ago

@tsteur I have quickly set up a draft, which should fully fix it (at least for comparison). It adds an additional column for the comparison metrics, that indicates the trend. As this is done prior to formatting the number, this should work for all languages.

tsteur commented 2 years ago

@sgiehl I'll put the issue for now in 4.10 as it's quite a big change and adds heaps more information to the API output etc.

If any possible, we should check if there's a smaller change that fixes this issue as well for Swedish maybe like in https://github.com/matomo-org/matomo/pull/18832/files#diff-9e27247dd682afe6f5fbc3c43bc37873fe2b5583a69ce15d233edfea3377aa3aR4 checking if the value contains the minusSign or translated minus sign for example. Then we could fix it sooner without needing to add so much information to the API and it would work in most countries.

sgiehl commented 2 years ago

If the additional api output is a problem, we can simply remove it by default and only add it for internal requests when needed

tsteur commented 2 years ago

Not having the additional API output makes it better. I've moved this for now into 4.10 as the PR is a bigger review and quite complex and we have other more important issues in comparison. If there's an easy solution to it, then happy to put it in an earlier milestone.