bugdone / headshotbox

https://headshotbox.github.io
MIT License
163 stars 25 forks source link

Added locale to toLocaleString in timestamp2Date #198

Closed ajohnsen closed 7 years ago

ajohnsen commented 7 years ago

This should fix the issue where Chrome prints out M01, M02 etc for the month name. Passing in an undefined locale causes this error.

bugdone commented 7 years ago

Which Chrome version are you using? I can't reproduce this bug.

ajohnsen commented 7 years ago

I think i've been using the latest, but i don't have the problem on Windows 10.. So it might be a problem with macOS.

bugdone commented 7 years ago

I'm reluctant to accept this pull request since using undefined seems to use the user's locale. Users with a different locale than en-US wouldn't appreciate this change.

ajohnsen commented 7 years ago

Yeah this issue seems weird... i did some testing, and it seems to be only in Chrome on mac..

Console testing:

d = new Date(1496167105*1000);
Tue May 30 2017 19:58:25 GMT+0200 (CEST)

format = {day: 'numeric', month: 'short'};
Object {day: "numeric", month: "short"}

d.toLocaleString(undefined, format)
"M05 30"

navigator.language
"en-GB"

d.toLocaleString('en-GB', format)
"30 May"
bugdone commented 7 years ago

Can you change the code so only on chrome/mac it forces the US locale?

pz1k commented 7 years ago

I have the same issue W10 & Chrome 58.x 64bits

ajohnsen commented 7 years ago

Ok, i'll try to see if i can get the same issue on windows 10 as well. I have an idea for a fix, but im not 100% sure if its supported in all browsers. So ill check that first, then make a new pull request for it.

bugdone commented 7 years ago

I assume the problem is either:

The first issue could be solved by always passing a locale string (by trying to get it like so and forcing the US locale if that yields undefined)

ajohnsen commented 7 years ago

I now added some code to find the browserlanguage that works for Chrome, FF, IE etc.

https://zzz.buzz/2016/01/13/detect-browser-language-in-javascript/

ajohnsen commented 7 years ago

Seems like this bug was fixed in Chrome 59, so you could think about reverting this change. Vivaldi has the same error and is using the same engine as Chrome, so i think its a bug in the engine that chrome got a fix for now, and vivaldi will probably get it soon.