Skip to content

[Bug report] lang on the 404 page is not set and defaults the Locale to "en" #1365

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

Closed
vordimous opened this issue Jun 21, 2023 · 11 comments · Fixed by #1380
Closed

[Bug report] lang on the 404 page is not set and defaults the Locale to "en" #1365

vordimous opened this issue Jun 21, 2023 · 11 comments · Fixed by #1380

Comments

@vordimous
Copy link
Contributor

Description

The 404 page is not getting normal page variables sent to it. This means the locale in js defaults to "en" when none is present.

image

This is causing js plugins like algolia search to get the wrong lang value. Which should be "en-US" instead of "en".

image

Code issue:

resolvePageLang: (page: PageData): PageLang => page.lang || 'en',

Potential fix:

resolvePageLang: (page: PageData): PageLang => page.lang || window.navigator.language || window.navigator.browserLanguage || window.navigator.userLanguage || 'en',

Reproduction

the base example v2.vuepress.vuejs.org/new shows the error when checking the value of 'document.documentElement.lang' from the home page and 404 page.

Used Package Manager

pnpm

System Info

npm ERR! code E404
npm ERR! 404 Not Found - GET https://registry.npmjs.org/zilla-docs - Not found
npm ERR! 404 
npm ERR! 404  'zilla-docs@*' is not in this registry.
npm ERR! 404 
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

npm ERR! A complete log of this run can be found in: /Users/adanelz/.npm/_logs/2023-06-21T13_52_42_634Z-debug-0.log
@Mister-Hope
Copy link
Member

The solution may break ssr as there is no window during ssr, and it can cause an ssr mismatch.

Have you tested your "Potential fix"?

@vordimous
Copy link
Contributor Author

I have not tested this fix with an ssr project. In the browser it does return the correct locale. Perhaps this project could shed some light on what is needed: https://vueuse.org/core/useNavigatorLanguage/

@vordimous
Copy link
Contributor Author

bump, any updates?

@meteorlxy
Copy link
Member

I think the page lang should clarify the language of the page content, instead of the browser settings.

@vordimous
Copy link
Contributor Author

The main problem is the static default and no way to configure it. The proposed solution still prefers the page set but also looks at the browser. Another option would be for a configurable fallback vs a static one.

I think the page lang should clarify the language of the page content instead of the browser settings.

The best option is for the 404 and all pages generated by vuepress to respect the lang setting. The notFound setting doesn't look to allow for multiple languages. Is that intentional?

@vordimous
Copy link
Contributor Author

Does anyone know if there is a workaround as this is actively making our search not work from the 404 page since everything is indexed against en-US.

@meteorlxy
Copy link
Member

@vordimous Have you tried to create a 404.md with frontmatter to customize it? (I'm not sure if I have documented it correctly 🤔

@vordimous
Copy link
Contributor Author

@meteorlxy I tried to create the 404.md, and when I render that page path specifically (/404.html) it will pull the frontmatter correctly as it renders it like any other page. However, vuepress is loading the 404 content dynamically for a missing link and not pulling the content from the 404.md file. Is there some extra config setting I need to point to the 404.md file?

@vordimous
Copy link
Contributor Author

bump

@Mister-Hope
Copy link
Member

This is an actual problem, see https://theme-hope.vuejs.press/zh/a and https://theme-hope.vuejs.press/a

I think we should respect locale setting, as you can see that those pages are rendering exactly as locale settings.

A valid PR is always welcome, I am busy these days.

@vordimous
Copy link
Contributor Author

vordimous commented Jul 10, 2023

@Mister-Hope I dug a little deeper and found a good solution. It turns out the problem is just in the computed resolver so there is much less of an impact. Here is my PR

meteorlxy pushed a commit that referenced this issue Jul 11, 2023

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
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 a pull request may close this issue.

3 participants