Bug: getBrowserCultureLang() is not working as expected in all browsers

MickL commented 3 years ago

Current behavior

Using getBrowserCultureLang() returns:

Chrome: de-DE (correct)
Firefox: de (wrong)
Safari: de-de (wrong)

The problem gets worse when using transloco-locale:


translocoLocaleService.setLocale(getBrowserCultureLang())

-> Throws "de isn't a valid locale format"

Expected behavior

Should always return correct format 'de-DE'

Minimal reproduction of the problem with instructions

import { getBrowserCultureLang } from '@ngneat/transloco';

console.log(getBrowserCultureLang());

Environment


- transloco: 2.20.1
- transloco-locale: 1.4.0
- Angular: 12.0.1

shaharkazaz commented 3 years ago

@MickL Can you please provide the browser versions that you tested this issue on?

MickL commented 3 years ago

Latest versions everywhere:

Chrome 91.0.4472.164 Safari 14.1.1 Firefox 90.0.1

dvdvnl commented 1 year ago

This issue is still open and reproducible with current browsers and transloco/transloco-locale versions. Are there any fixes available @shaharkazaz?

shaharkazaz commented 1 year ago

@dvdvnl Currently no. You are welcome to open a PR for it 🙂

ld210 commented 1 year ago

const navigator = window.navigator;
return navigator.languages ? navigator.languages.find((lang) => lang.indexOf('-') > -1 || lang.indexOf('_') > -1) : navigator.language;

... should yield consistent results across browsers. Well, at least Chrome/Firefow/Safari

shaharkazaz commented 3 weeks ago

After @rraziel opened the PR, I started digging deeper into the issue.

The main problem isn't browser consistency— all the values emitted by the browsers are valid, as they follow BCP 47. Both Safari and Chrome produce the same values, which makes sense because according to this section of BCP 47:

At all times, language tags and their subtags, including private use and extensions, are to be treated as case insensitive.

In other words, everything is case insensitive.

Firefox is also valid, as this section clarifies:

A language tag is composed of a sequence of one or more "subtags."

One subtag (the primary language in this case) is valid and is listed in the appendix.

This brings me to a few key points:

isLocaleFormat is incorrectly implemented because it doesn't actually support BCP 47 formats (the root cause of the issue).
getBrowserCultureLang was never intended to return a unified string. It might be worth considering deprecating this function, as it only returns window.navigator.language (window.navigator.languages[0] is redundant, since they are the same). The one advantage of this function is its consideration of SSR by returning an empty string, which could save some code for Transloco users.

To resolve this issue, I'll update the BCP 47 validator to correctly validate the format. Feel free to share thoughts, suggest a different solution or give another perspective.

jsverse / transloco