Default Accept-Language to I18n.default_locale
#50
Conversation
Currently AutoLocale tries to set `I18n.locale` to nil when the Accept-Language header is missing, which leads to "nil is not a valid locale" error when `I18n.config.enforce_available_locales` is true (default). This commit fixes the problem by defaulting the header value to `I18n.default_locale`.
| I18n.locale = http_accept_language.compatible_language_from(I18n.available_locales) | ||
| hal = http_accept_language | ||
| hal.header ||= I18n.default_locale.to_s | ||
| I18n.locale = hal.compatible_language_from(I18n.available_locales) |
There was a problem hiding this comment.
Would this work as well?
I18n.locale = http_accept_language.compatible_language_from(I18n.available_locales) || I18n.default_localeThere was a problem hiding this comment.
@DouweM Thanks for taking a look! I'm afraid it wouldn't, because compatible_language_from may return nil when the Accept-Language header is given and contains no available locales. Applying a fallback to those cases as well will spoil the enforce_available_locales concept.
There was a problem hiding this comment.
@knu I'm not sure I understand. If none of the locales in Accept-Language are supported by the website, why wouldn't you want to fall back to the default locale?
With your code, I18n.locale can still end up being set to nil, which, as your write, leads to a "nil is not a valid locale" error.
There was a problem hiding this comment.
That is expected behavior when enforce_available_locales is set to true and the client only lists unsupported locales, isn't it?
There was a problem hiding this comment.
In this case, I would assume you would want to still present your website to the user using the default locale (usually English) rather than error out.
There was a problem hiding this comment.
OK, it's your call to decide how HttpAcceptLanguage::AutoLocale behaves. You could easily implement your own variant to meet your needs anyway, like refusing a fallback by returning 406, adding a Content-Language response header, etc.
|
Is there any concerns with merging this? /cc @DouweM |
Default `Accept-Language` to `I18n.default_locale`
|
Thanks @knu! |
|
@DouweM hey, could you please release a new version with this fix? Thanks! |
Currently AutoLocale tries to set
I18n.localeto nil when the Accept-Language header is missing, which leads to "nil is not a valid locale" error whenI18n.config.enforce_available_localesis true (default).This commit fixes the problem by defaulting the header value to
I18n.default_locale.