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

ACCEPT_LANGUAGE goes before default_langauge #1259

Merged
merged 2 commits into from
Sep 5, 2016
Merged

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Sep 4, 2016

See #970

Before we had

  1. Users settings in personal settings
  2. Admins default language settings
  3. Accept-Language settings of the browser

However this is not in line with
https://www.w3.org/International/questions/qa-lang-priorities

So this changes the order to

  1. Users settings in personal settings
  2. Accept-Language settings of the browser
  3. Admins default language settings

@kohtala @MorrisJobke please have a look

See #970

Before we had

1. Users settings in personal settings
2. Admins default language settings
3. Accept-Language settings of the browser

However this is not in line with
https://www.w3.org/International/questions/qa-lang-priorities

So this changes the order to

1. Users settings in personal settings
3. Accept-Language settings of the browser
2. Admins default language settings
@rullzer rullzer added enhancement 3. to review Waiting for reviews labels Sep 4, 2016
@rullzer rullzer added this to the Nextcloud 11.0 milestone Sep 4, 2016
@mention-bot
Copy link

@rullzer, thanks for your PR! By analyzing the annotation information on this pull request, we identified @nickvergessen, @LukasReschke and @MorrisJobke to be potential reviewers

@MorrisJobke
Copy link
Member

Tested and works 👍

@nickvergessen
Copy link
Member

👍

@nickvergessen nickvergessen merged commit db6a336 into master Sep 5, 2016
@nickvergessen nickvergessen deleted the language_order branch September 5, 2016 10:37
@MorrisJobke
Copy link
Member

@rullzer original issue was filled against 10.0.1 ... should this be backported?

cc @karlitschek

@rullzer
Copy link
Member Author

rullzer commented Sep 5, 2016

I don't really think so.

@karlitschek
Copy link
Member

please backport 👍

@karlitschek
Copy link
Member

@rullzer Why not?

@nickvergessen
Copy link
Member

It's somewhat a behaviour change. Not sure if we should backport this either. On the other hand for registered users it doesn't change anything. So maybe we could backport a "simpler" version of the patch

@rullzer
Copy link
Member Author

rullzer commented Sep 5, 2016

Exactly it changes behaviour. Ok fair enough it changes it to the correct one. But it is not a bug that leads to dataloss etc. Backporting always brings a risk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants