Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LanguageAccept.best_match() - better fallback #450

Closed
ThiefMaster opened this issue Oct 18, 2013 · 12 comments · Fixed by #1507
Closed

LanguageAccept.best_match() - better fallback #450

ThiefMaster opened this issue Oct 18, 2013 · 12 comments · Fixed by #1507
Milestone

Comments

@ThiefMaster
Copy link
Member

@ThiefMaster ThiefMaster commented Oct 18, 2013

>>> parse_accept_header('en-us', LanguageAccept).best_match(['en', 'de'])
>>> parse_accept_header('en', LanguageAccept).best_match(['en-us', 'de'])

Both those calls return None. I'm not sure if there's a RFC specifying the behaviour for this but shouldn't a territory-less language always work if there's no exact match available? I.e. if the client specified en-us, en would be fine and more important, if the client specified en and the server just knows en-us that one should be used.

This is important in cases like the following one: There's an application using Flask-Babel which has english as its native language (i.e. hardcoded) because it's nice to keep code in english and a translation, e.g. german. That language is the default language though (because the main audience of the site speaks german). However, if a client sends an Accept-Languages for en (or any en-*) the english "translation" should be used. But without specifying en and all en-* language codes when calling best_match() it will return None (or de depending on the passed arguments) instead - even though the user's header clearly says he prefers english.

@ThiefMaster
Copy link
Member Author

@ThiefMaster ThiefMaster commented Oct 18, 2013

Babel has a function negotiate_locale which does exactly what I'm suggesting here.

@babel.localeselector
def get_current_locale():
    preferred = [x.replace('-', '_') for x in request.accept_languages.values()]
    return negotiate_locale(preferred, AVAILABLE_LOCALES)

Since the function is rather big it probably wouldn't be a good idea to include it in werkzeug. Might be worth mentioning it in the docs of Flask-Babel though since it's more useful than the current example using best_match().

@budlight
Copy link

@budlight budlight commented Apr 30, 2014

This leaves out matching based on quality though. So this solution isn't technically correct. I'm not really sure whether it is preferred for the fix to be in werkzeug or in babel. best_match does do quality matching, but babel doesn't.

@lassegit
Copy link

@lassegit lassegit commented May 18, 2016

I had problems matching various german accept_languages, foreinstance de-de, de-at. This seems to work for me (all de-de or de-at will be 'de'):

`

@babel.localeselector
def get_locale():
    locale = 'en'

    for lang in request.accept_languages.values():
        if lang[:2] in ['de', 'en']:
            locale = lang[:2]
            break

    g.locale = locale
    return locale

`

@lafrech
Copy link
Contributor

@lafrech lafrech commented Sep 15, 2016

Babel has a function negotiate_locale which does exactly what I'm suggesting here.

@babel.localeselector
def get_current_locale():
    preferred = [x.replace('-', '_') for x in request.accept_languages.values()]
    return negotiate_locale(preferred, AVAILABLE_LOCALES)

I'm not sure this covers every case.

If my application defines locales as lang_territory and the browser sends a request as lang_territory where only lang matches:

from babel import negotiate_locale
AVAILABLE_LOCALES = ['en_US', 'de_DE']
negotiate_locale(['en_EN'], AVAILABLE_LOCALES)  # returns None

Isn't this the typical use case?

Or am I missing something?

@arthurwhite
Copy link

@arthurwhite arthurwhite commented Oct 14, 2016

This leaves out matching based on quality though. So this solution isn't technically correct. I'm not really sure whether it is preferred for the fix to be in werkzeug or in babel. best_match does do quality matching, but babel doesn't. — @budlight

No, the @ThiefMaster snippet is correct as an Accept object is automatically sorted by quality (from 1 to 0) so negotiate_locale tries to match the most preferred first.

The @lafrech case also should work.
But that's a Babel issue.

@kramred
Copy link

@kramred kramred commented May 10, 2017

Just stumbled upon this behaviour and thought/think it is a bug, as it does not fall back to e.g. 'de' if only 'de-AT' is in request.accept_languages.
I tracked it down to the
class LanguageAccept(Accept): in datastructures.py (see here)
where I think it should be return _locale_delim_re.split(language.lower())[0] with the [0] added inside
def _value_matches(self, value, item): def _normalize(language):
Will add more info later, see this gist for testing
Edit1: adjusted the gist to include the examples from first post and I think it does what I want it to do – should I maybe create a pull req. or could you add the fix after further testing?

@arthurwhite
Copy link

@arthurwhite arthurwhite commented May 10, 2017

I've already submitted a PR: #1020. ⌛️

@TimotheeJeannin
Copy link

@TimotheeJeannin TimotheeJeannin commented May 25, 2018

Any updates on this?

@lepture
Copy link
Contributor

@lepture lepture commented May 25, 2018

I thought #1065 has done the work. cc @ThiefMaster

@fossilet
Copy link

@fossilet fossilet commented Sep 4, 2018

Though #1065 merged, this still persist in 0.14.1. I am now using babel. negotiate_locale as a workaround.

@chivalry
Copy link
Contributor

@chivalry chivalry commented Apr 24, 2019

I've submitted a PR for this, would love to get feedback.

#1507

@Dreamsorcerer
Copy link

@Dreamsorcerer Dreamsorcerer commented Jul 1, 2019

The negotiate_locale() workaround doesn't even work as expected by this thread. e.g.
negotiate_locale(('en',), ('en_gb', 'de'))
Returns None. The original example with en_us only works due to it being an alias for en in the default kwargs.

It also doesn't know how to handle more extended forms of locales, such as those including the script (e.g. zh_Hant_TW).

@davidism davidism added this to the 1.0.0 milestone Jul 14, 2019
abompard added a commit to fedora-infra/noggin that referenced this issue Feb 19, 2020
And remove some locale-handling code that has been implemented upstream.
(pallets/werkzeug#450)

Signed-off-by: Aurélien Bompard <aurelien@bompard.org>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.