Skip to content

User language control in the server #849

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

Merged
merged 9 commits into from
Dec 14, 2022
Merged

Conversation

veloman-yunkan
Copy link
Collaborator

@veloman-yunkan veloman-yunkan commented Nov 28, 2022

Fixes #750

Depends on #848

kiwix server determines the user language from three sources (in that order, earlier source takes precedence):

  • userlang URL query parameter
  • userlang cookie
  • Accept-Language header

In all cases, the server sets the userlang cookie in its response using thus determined language.

If Accept-Language is used and multiple language options are specified with weights, then the most suitable language is selected by scoring available translation languages - the score value is computed as the product of weight value and the number of translated strings for that language.

@veloman-yunkan veloman-yunkan changed the base branch from master to fake_language_for_i18n_testing November 28, 2022 09:58
@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Base: 71.04% // Head: 71.04% // No change to project coverage 👍

Coverage data is based on head (aa7053b) compared to base (aa7053b).
Patch has no changes to coverable lines.

❗ Current head aa7053b differs from pull request most recent head 28e9fb4. Consider uploading reports for the commit 28e9fb4 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #849   +/-   ##
=======================================
  Coverage   71.04%   71.04%           
=======================================
  Files          53       53           
  Lines        3685     3685           
  Branches     2058     2058           
=======================================
  Hits         2618     2618           
  Misses       1065     1065           
  Partials        2        2           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@veloman-yunkan veloman-yunkan marked this pull request as ready for review November 29, 2022 10:20
@veloman-yunkan
Copy link
Collaborator Author

Note that in this version dialects/language variants (like en-US, fr-XVI, etc) are treated as regular languages - no attempt to convert them into language codes that are available in our translation database is performed.

@veloman-yunkan veloman-yunkan force-pushed the backend_userlang_control branch from acd85aa to d383ee8 Compare November 29, 2022 10:37
Base automatically changed from fake_language_for_i18n_testing to master November 29, 2022 15:19
Copy link
Member

@mgautierfr mgautierfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small question/update but we are mostly good.

@kelson42
Copy link
Collaborator

kelson42 commented Dec 7, 2022

Note that in this version dialects/language variants (like en-US, fr-XVI, etc) are treated as regular languages - no attempt to convert them into language codes that are available in our translation database is performed.

Note that in this version dialects/language variants (like en-US, fr-XVI, etc) are treated as regular languages - no attempt to convert them into language codes that are available in our translation database is performed.

What is the user impact of that, this seems important that fr-CH delivers fr in case we don't have any fr-CH translations.

@kelson42
Copy link
Collaborator

kelson42 commented Dec 7, 2022

It seems here, no javascript file is changed, so I wonder about my comment #750 (comment). How should we do, to change the code which handle the langauge selection box from javascript to c++?

@veloman-yunkan
Copy link
Collaborator Author

What is the user impact of that, this seems important that fr-CH delivers fr in case we don't have any fr-CH translations.

@kelson42 I would prefer to address that in a separate PR (after this one and #846 are merged).

@kelson42
Copy link
Collaborator

kelson42 commented Dec 8, 2022

What is the user impact of that, this seems important that fr-CH delivers fr in case we don't have any fr-CH translations.

@kelson42 I would prefer to address that in a separate PR (after this one and #846 are merged).

Can you please open a ticket for that? Otherwise it will be forgotten until a user complain about it.

@veloman-yunkan
Copy link
Collaborator Author

It seems here, no javascript file is changed, so I wonder about my comment #750 (comment). How should we do, to change the code which handle the langauge selection box from javascript to c++?

@kelson42 That is addressed in #846.

@mgautierfr mgautierfr force-pushed the backend_userlang_control branch from d703ba8 to 28e9fb4 Compare December 14, 2022 14:34
@mgautierfr
Copy link
Member

Rebased on master and fixed-up.
I'm merging.

@mgautierfr mgautierfr merged commit a10067e into master Dec 14, 2022
@mgautierfr mgautierfr deleted the backend_userlang_control branch December 14, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Correctly parse Accept-Language header.
3 participants