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

Provide a fallback for LanguageAccept under Safari #1507

Merged
merged 3 commits into from Sep 5, 2019

Conversation

@chivalry
Copy link
Contributor

@chivalry chivalry commented Apr 6, 2019

Safari on macOS doesn't provide a language-only option in the header for languages accepted. When en-US is the preferred language, Safari only provides an "Accepted Language" of en-US without any fallback to en. Therefore, if a website is offering an en language version but not an en-US version, LanguageAccept.best_match returns None.

This pull request attempts to correct that by checking if None is returned and if so, trying again with language-only accepted values.

fixes #450

@chivalry
Copy link
Contributor Author

@chivalry chivalry commented Apr 8, 2019

It appears this is a known issue? #450

It looks like there might be a better way to do this than what I came up with. I'll do a bit more research when I have time. For the time being my app is using my own function instead of best_match.

@chivalry
Copy link
Contributor Author

@chivalry chivalry commented Apr 24, 2019

After some research, I don't see a better way to handle this, but I did add some additional tests.

@chivalry
Copy link
Contributor Author

@chivalry chivalry commented Apr 24, 2019

Have the tests become more strict? Did I miss something? The tox app says there are failures in tests/test_test.py and src/werkzeug/debug/__init__.py, but my commit's version of those files is the same as in pallets:master.

@davidism
Copy link
Member

@davidism davidism commented Apr 25, 2019

You need to follow the style checks, in this case ensure you use Black to format the code. You can see the changes it wants to make here: https://travis-ci.org/pallets/werkzeug/jobs/524209888#L219-L262 You can install pre-commit to have this done automatically before commits.

@davidism
Copy link
Member

@davidism davidism commented Apr 25, 2019

There are three issues related to accept matching: #20, #450, and #458. I think the first comment in #20 may be listing the other two, but it would be good to review them.

@davidism davidism requested a review from ThiefMaster Apr 25, 2019
src/werkzeug/datastructures.py Outdated Show resolved Hide resolved
src/werkzeug/datastructures.py Outdated Show resolved Hide resolved
@davidism davidism added this to the 1.0 milestone May 12, 2019
@davidism davidism removed the request for review from ThiefMaster Jul 14, 2019
@davidism davidism force-pushed the lang-accept-fallback branch from d89e73d to a61e6d6 Jul 14, 2019
@davidism
Copy link
Member

@davidism davidism commented Jul 14, 2019

This doesn't go the opposite direction, where the client accepts "en", but the server looks for "en_US". Is that intended / correct?

a = LanguageAccept((("en", 0.9),))
m = a.best_match(("en_US",))
print(m)  # None

chivalry and others added 3 commits Sep 5, 2019
If no exact match is found, first tries modifying the accepted values to
use primary tags only, then tries modifying the matched values to use
primary tags only.

If the client only accepts "en-US", "en" will match. If the client only
accepts "en", "en-US" will match. 2 and 3 lettter codes are supported.
Fallback matching is not performed with other subtags.
@davidism davidism force-pushed the lang-accept-fallback branch from a61e6d6 to 9bfca43 Sep 5, 2019
@davidism
Copy link
Member

@davidism davidism commented Sep 5, 2019

Added a fallback in the other direction. The process is now:

  1. Exact match
  2. Accept on primary, if the client accepts "en-US", "en" matches
  3. Match on primary, if the client accepts "en", "en-US" matches.

Also fixed to match 3-letter primary tags, like "aus".

Given the way the code is written, you could imagine a fourth fallback where the primary tag is considered for both the accept and match values at the same time. This doesn't make sense however, as "en-GB" would then match "en-US".

@davidism davidism requested review from ThiefMaster and removed request for ThiefMaster Sep 5, 2019
@davidism davidism merged commit 01f8756 into pallets:master Sep 5, 2019
11 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
@chivalry chivalry deleted the lang-accept-fallback branch Apr 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants