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

Reopen #1999: Can't resolve the locale #2097

Closed
gnh1201 opened this issue Feb 10, 2023 · 4 comments · Fixed by #2102
Closed

Reopen #1999: Can't resolve the locale #2097

gnh1201 opened this issue Feb 10, 2023 · 4 comments · Fixed by #2102
Assignees
Labels
area/i18n/translations effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful kind/bug A bug in existing code (including security flaws) need/analysis Needs further analysis before proceeding P1 High: Likely tackled by core team if no one steps up

Comments

@gnh1201
Copy link

gnh1201 commented Feb 10, 2023

Describe the bug
Can't resolve the locale (Korean, ko, ko-KR), like #1999

To Reproduce
If I trying connect the IPFS webui on my computer (with ko-KR locale), it tries to access the wrong address.

Expected behavior
It connect to the locale ko-KR. /ipfs/[BAFYHASH]/locales/ko-KR/[FILENAME].json

Actual behavior
It connect to the locale ko (it does not exist). /ipfs/[BAFYHASH]/locales/ko/[FILENAME].json
As a result, I got this error:

ipfs resolve -r /ipfs/[BAFYHASH]/locales/ko/settings.json: no link named "[FILENAME].json" under [BAFYHASH]

the ko and the ko-KR are the same code. It need a proper redirection.

@gnh1201 gnh1201 added the need/triage Needs initial labeling and prioritization label Feb 10, 2023
@gnh1201 gnh1201 changed the title Reopen: Can't resolve the locale Reopen #1999: Can't resolve the locale Feb 10, 2023
@whizzzkid whizzzkid added kind/bug A bug in existing code (including security flaws) need/analysis Needs further analysis before proceeding need/author-input Needs input from the original author area/i18n/translations and removed need/triage Needs initial labeling and prioritization labels Feb 17, 2023
@whizzzkid
Copy link
Contributor

The redirection logic introduced in #2005 seems to exist https://github.com/ipfs/ipfs-webui/blob/main/src/i18n.js#L42.

I am wondering if #2092 caused a regression.

CC: @SgtPooki @hacdias

@SgtPooki SgtPooki added P0 Critical: Tackled by core team ASAP exp/intermediate Prior experience is likely helpful effort/hours Estimated to take one or several hours P1 High: Likely tackled by core team if no one steps up and removed need/author-input Needs input from the original author P0 Critical: Tackled by core team ASAP labels Feb 21, 2023
@SgtPooki SgtPooki self-assigned this Feb 22, 2023
@SgtPooki SgtPooki linked a pull request Feb 23, 2023 that will close this issue
@SgtPooki
Copy link
Member

SgtPooki commented Feb 23, 2023

FYI that I added some tests for i18n, and have a proper test case that is failing to prove this bug. working on fix now.

edit: failing testcase: https://github.com/ipfs/ipfs-webui/actions/runs/4248889633/jobs/7388520133#step:7:351

@SgtPooki
Copy link
Member

I can repro the failure when loading via IPFS url at ipfs://bafybeidxhsjdowd3kgekefq3ezpcektrq7i2i4dnbrrioiwftvksz4oof4/#/welcome even with en-US locales:

image

the above CID is for a local build I did:

> npm run build
...
> ipfs add --cid-version 1 -Q -r ./build
bafybeidxhsjdowd3kgekefq3ezpcektrq7i2i4dnbrrioiwftvksz4oof4

looking into modifying the i18n-http-backend loadPath option so that we are only sending requests for the locales that exist

@SgtPooki
Copy link
Member

SgtPooki commented Feb 23, 2023

@gnh1201 Can you confirm that things are working properly with the build from the PR build ?

Things are loading properly for me, with no misses on locale fetches any longer.

PR: #2102
PR build: https://github.com/ipfs/ipfs-webui/actions/runs/4257768580/jobs/7408314042
CID: bafybeic5stlj7yotwlhy4f4nrcf2g2agtmugey7yc34itjt3qe4bekx3hq
IPFS hosted webui: ipfs://bafybeic5stlj7yotwlhy4f4nrcf2g2agtmugey7yc34itjt3qe4bekx3hq/#/settings
fleek preview link: https://bafybeihu3jajpv2awi73mgdxhvkswi6n3dzxej4dhzkdafw2rwpztbkrqm.on.fleek.co/

why the fleek CID and github CI's CID are different are unknown to me. probably because fleek is bundling up more than just the build dir?

SgtPooki added a commit that referenced this issue Feb 25, 2023
* test: test i18n languages and fallbacks

* test(i18n): use available port

* chore: add all local files for ko-KR

english has 8 locale files:
`ls -lhatr public/locales/en/*.json  | wc -l # 8`

before this change, ko-KR only had 4
`ls -lhatr public/locales/ko-KR/*.json  | wc -l # 4`

so I copied them over using
`cp -n public/locales/en/*.json public/locales/ko-KR/`

after this change
`ls -lhatr public/locales/ko-KR/*.json  | wc -l # 8`

* chore(i18n): ensure app:actions.add has ko translation

* fix: only send i18n requests for current language

Sends only a single request for lang via i18n-http-backend

see i18next/i18next-http-backend#61

* fix: current language displays correctly for fallbacks

* test(lib/i18n): test getLanguage function

* test(lib/i18n): add getCurrentLanguage test

* test(i18n): add test for naming languages in languages.json

* fix(i18n): add parser for getting valid locale codes

* fix(lib/i18n): use i18n-localeParser

* fix(i18n): prevent the lookup of invalid locales

fixes #2097

* test(e2e:settings): test language selector

* test(e2e/settings): validate that language files are requested

* chore: remove untranslated ko-KR files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/i18n/translations effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful kind/bug A bug in existing code (including security flaws) need/analysis Needs further analysis before proceeding P1 High: Likely tackled by core team if no one steps up
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants