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

Change browser language detection to only redirect from default locale route #455

Closed
rchl opened this issue Sep 16, 2019 · 9 comments · Fixed by #799
Closed

Change browser language detection to only redirect from default locale route #455

rchl opened this issue Sep 16, 2019 · 9 comments · Fixed by #799

Comments

@rchl
Copy link
Collaborator

rchl commented Sep 16, 2019

What problem does this feature solve?

When using strategies with prefixes, and detectBrowserLanguage enabled, when user goes directly to a route with non-default locale, it would be better if we didn't redirect to browser's locale. The user's intention, in theory, was to visit the site in that specific locale

Some more context in #451

What does the proposed changes look like?

Only redirect and change locale if first navigation is to route that matches defaultLocale.

That would be a breaking change and would require major version.

This feature request is available on Nuxt community (#c301)
@laurentdebricon
Copy link

someone has a fix ?

@konr4d
Copy link

konr4d commented Nov 13, 2019

@laurentdebricon A quick workaround konr4d@37f394d

Edit: I was able to make nuxt-i18n redirect from root path only and respect the saved cookie. E.g., a new user from France is initially redirected to /fr per hers/his Accept-Language. Then, every / entry results in a redirect to a locale path based on the value in that cookie.
I wish I had time to write a proper non-kludge code and form it into a PR. Alas, for now it lives but here: https://github.com/konr4d/nuxt-i18n/tree/only-root-redirect

@stale
Copy link

stale bot commented Jan 18, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 18, 2020
@rchl rchl removed the wontfix label Jan 19, 2020
@urbgimtam
Copy link

This issue behaviour should be implement by default, as currently this module can crash a website when using the final segment of the url as a variable to fetch data, which is a normal pattern in vue and nuxt.

Example:
EN: https://example.com/en/pages/this-is-a-page
PT: https://example.com/pt/paginas/esta-e-uma-pagina

In English, 'this-is-a-page' is used to fetch the contents to the database.
In Portuguese, 'esta-e-uma-pagina' is used to fetch the contents to the database.

As it stands, if the user opens the page https://example.com/pt/paginas/esta-e-uma-pagina but its browser autodetects its English, it will update the route to https://example.com/en/pages/esta-e-uma-pagina. However, this page does not exist, thus crashing, or displaying the content in a different language.

IMO, the user should see the content of the link he clicked. That's what he expects when he clicks, let's say, on a social media shared post. If he wants to change language afterwards, its his call (sometimes the translated content might even not exist in the same fashion, or different services are provided in different countries).

The workaround from @konr4d is awesome, and solves this issue. I never done a PR, but I'll make one with @konr4d code (if he doesn't mind), as this issue has affected me already a few times.

@rchl
Copy link
Collaborator Author

rchl commented Feb 19, 2020

@urbgimtam Why is it redirecting to invalid https://example.com/en/pages/esta-e-uma-pagina route? Isn't that the main problem in your case? I think the problem I've described in this issue is different from yours (even if fix for this issue would likely also fix your case).

@urbgimtam
Copy link

@rchl This is a colateral damage/another symptom of the same issue.

In my example, if the users browser is english but he visits the portuguese page url, it is expected to result in the portuguese page, exactly like you say.

However, the current status of nuxt-i18n does not behave like that. It tries to detect the browsers language and redirect accordingly, and by doing so, it overrides the language present in the url.

What I tried to say was that this behavior, in conjunction with the common pattern of using the last segment of the url as a variable in vue/nuxt, ends up generating errors.

In your case, from what I understand, is the same issue: the user wants to go to a non-default locale, but the detectBrowserLanguage takes over, generating the language cookie AND redirecting to that detected language.

The solution presented by @konr4d allows that behavior in the following fashion:

In root path:

  • If a language is not requested on the root path, the detectBrowserLanguage generates the correct cookie and redirects
  • If a language is requested on the root path, the detectBrowserLanguage is bypassed and no redirection occurs.

In any other path:

  • If a language is not requested on the path (no "prefix", for example) it assumes the default fallback language and presents the page.
  • if a language is requested on the path (the prefix is present on the url), it uses that language as the current language and presents that page.

The main difference is that there's no redirection, and the full path is honored.

@stale
Copy link

stale bot commented Apr 19, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Apr 19, 2020
@rchl rchl removed the wontfix label Apr 19, 2020
@bnss
Copy link

bnss commented Jun 18, 2020

I can see that there is a pr which would fix/change this behaviour (#603). Any idea when/if this will be included?

rchl added a commit that referenced this issue Jul 20, 2020
Don't attempt to detect browser's language when user (or crawler) goes
directly to a page with a locale prefix. This is to avoid redirecting
user that potentially wanted to load that specific page rather than the
one matching browser's language.

Resolves #455
Resolves #761
rchl added a commit that referenced this issue Jul 20, 2020
Don't attempt to detect browser's language when user (or crawler) goes
directly to a page with a locale prefix. This is to avoid redirecting
user that potentially wanted to load that specific page rather than the
one matching browser's language.

Resolves #455
Resolves #761
rchl added a commit that referenced this issue Jul 20, 2020
Don't attempt to detect browser's language when user (or crawler) goes
directly to a page with a locale prefix. This is to avoid redirecting
user that potentially wanted to load that specific page rather than the
one matching browser's language.

Resolves #455
Resolves #761
rchl added a commit that referenced this issue Sep 1, 2020
…ocale

Don't attempt to detect browser's language when user (or crawler) goes
directly to a page with a locale prefix if that locale is not the default
one. This is to avoid redirecting user that potentially wanted to load
that specific page rather than the one matching browser's language.

Resolves #455
Resolves #761
rchl added a commit that referenced this issue Sep 9, 2020
…ocale

Don't attempt to detect browser's language when user (or crawler) goes
directly to a page with a locale prefix if that locale is not the default
one. This is to avoid redirecting user that potentially wanted to load
that specific page rather than the one matching browser's language.

Resolves #455
Resolves #761
@rchl rchl closed this as completed in #799 Sep 9, 2020
rchl added a commit that referenced this issue Sep 9, 2020
Don't attempt to detect browser's language when user (or crawler) goes
directly to a page with a locale prefix if that locale is not the default
one. This is to avoid redirecting user that potentially wanted to load
that specific page rather than the one matching browser's language.

Resolves #455
Resolves #761
@rchl
Copy link
Collaborator Author

rchl commented Sep 10, 2020

Fix released in v6.15.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants