432847 - Character corruption in download count

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Localization, defect)

Description

[Ryan Doherty (:rdoherty)]

Attachment: Screenshot

I see a character being corrupted in the download counts for RU.

Comment 1

[Vlado Valastiak [:wladow]]

This affects also cs, fi, sk etc.

Comment 2

[Fred Wenzel [:wenzel]]

http://php.net/number_format suggests that the function number_format() used here is not multibyte-safe. However the comment is from 2006 so I am not sure if this persists but this may well be causing the problem seen here.

Comment 4

[Fred Wenzel [:wenzel]]

Attachment: multibyte-safe number_format

I wrote a little workaround for a multi-byte safe number_format function. It first uses '.' and ',' characters, then str_replace()s the actual characters in. Seems to work just fine with Russian.

Comment 5

[Fred Wenzel [:wenzel]]

Comment on attachment 322494 [details] [diff] [review]
multibyte-safe number_format

Not sure if Ryan is around to look at this? But it should be quite easy to review/test. Thanks.

Comment 6

[Ryan Doherty (:rdoherty)]

I can review later this week if necessary.

Comment 7

[Fred Wenzel [:wenzel]]

(In reply to comment #6)
> I can review later this week if necessary.
> 

Yes please, it's a small fix, but immediately publicly obvious on every page for the people using these locales, so I think we should get it fixed.

Comment 8

[Michael Morgan [:morgamic]]

Comment on attachment 322494 [details] [diff] [review]
multibyte-safe number_format

Me likey -- works for me, and as expected for the locales I looked at.

Comment 9

[Michael Morgan [:morgamic]]

Comment on attachment 322494 [details] [diff] [review]
multibyte-safe number_format

BTW - you will have to unbitrot your helper function.

Comment 10

[Fred Wenzel [:wenzel]]

Oh yeah, I fixed the conflict before checking it in. trunk had mildly outgrown this patch ;)

It's in r14159.

Thanks!

Comment 11

[Fred Wenzel [:wenzel]]

I need to reopen this: According to http://fredericiana.com/?p=1287&cp=1#comment-156454 I need to reopen this because the function won't work for the specific case when you format a number with decimals, and use commas as digit group separator and a point as the decimal separator.

It's due to str_replace first replacing commans by points, and then replacing *all* points by commas afterwards.

Replacing the placeholders by something uncommon may fix this, or using a regular expression function.

Comment 12

[Fred Wenzel [:wenzel]]

I changed the placeholders in r14187.

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using https://preview.addons.mozilla.org/ru/firefox/addon/3780, as well as the front, add-on detail, search, and browse pages of the cs, fi, and sk locales, where I spot-checked (comment 1).

Comment 14

[Fred Wenzel [:wenzel]]

FYI: Wladimir Palant commented on my blog mentioning that the whole "grouping numbers in 3" thing does not apply to some locales, including Japanese. So while this bug makes things look prettier, it's not quite "correct" yet. But that's beyond the scope here and probably not a high priority right now.

Comment 15

[Vlado Valastiak [:wladow]]

push-needed keyword removed, but I still see this using live page https://addons.mozilla.org/sk/firefox/. REOP?

Comment 16

[Fred Wenzel [:wenzel]]

Dang it, apparently I fixed all occurrences of number_format except that one. (Vlado: Check the search results page, for example). Reopening.

Comment 17

[Fred Wenzel [:wenzel]]

Fixed in r15531. Sorry I missed this.

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

/me chides self for missing the homepage before; verified FIXED now, though, on
 * https://preview.addons.mozilla.org/sk/firefox/
* https://preview.addons.mozilla.org/ru/firefox/
* https://preview.addons.mozilla.org/cs/firefox/
* https://preview.addons.mozilla.org/fi/firefox/

Comment 19

[Fred Wenzel [:wenzel]]

Thanks, Stephen! I felt kind of embarrassed too, missing the most obvious spot of all ;)

Comment 20

[Justin Scott [:fligtar]]

3.4.5 was pushed on Friday.