Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

fix template based i18n support issues #2259

Merged
merged 2 commits into from
May 15, 2022

Conversation

sio4
Copy link
Member

@sio4 sio4 commented May 5, 2022

During upgrading my old app, I found some i18n features related to the templates were not working correctly. The issues are:

  • When the r.HTML(string) stays with the old format, such as r.HTML("index.html"), but the template files are forms of index.plush.html and index.plush.ko-kr.html (renamed by buffalo fix), always the last file is used even though the browser's accept-language has no ko or ko-KR.
    • It happens because the logic of templateRenderer.updateAliases() does something unintended.
  • Also, it is not a functional issue but templateRenderer.resolve() is called twice in many cases since .localizedName() uses this for checking if there is a template for possible languages.

Test environment

  • template files:
    • templates/index.plush.html
    • templates/index.plush.ko-kr.html

Debugging result when using return c.Render(http.StatusOK, r.HTML("index.plush.html"))
--> OK, the correct templates are determined by the direct name match.

For ko-KR:

1. languages: [ko-KR ko en en-US en-US]
3. `index.plush.ko-kr.html`: try to resolve...
4. `index.plush.ko-kr.html` was **resolved directly** by the name
2. `index.plush.html` was localized as `index.plush.ko-kr.html`
3. `index.plush.ko-kr.html`: try to resolve...
4. `index.plush.ko-kr.html` was **resolved directly** by the name
3. `application.ko-kr.html`: try to resolve...
3. `application.ko.html`: try to resolve...
3. `application.en.html`: try to resolve...
2. `application.html` was localized as `application.html`
3. `application.html`: try to resolve...
5. `application.plush.html` was *resolved by map* for `application.html`
3. `_flash.html`: try to resolve...
5. `_flash.plush.html` was *resolved by map* for `_flash.html`

For the default language:

1. languages: [en en-US]
3. `index.plush.en.html`: try to resolve...
2. `index.plush.html` was localized as `index.plush.html`
3. `index.plush.html`: try to resolve...
4. `index.plush.html` was **resolved directly** by the name
3. `application.en.html`: try to resolve...
2. `application.html` was localized as `application.html`
3. `application.html`: try to resolve...
5. `application.plush.html` was *resolved by map* for `application.html`
3. `_flash.html`: try to resolve...
5. `_flash.plush.html` was *resolved by map* for `_flash.html`

Debugging result when using return c.Render(http.StatusOK, r.HTML("index.html"))
--> incorrect file is selected by alias matching.

For ko-KR

1. languages: [ko-KR ko en en-US en-US]
3. `index.ko-kr.html`: try to resolve...
3. `index.ko.html`: try to resolve...
3. `index.en.html`: try to resolve...
2. `index.html` was localized as `index.html`
3. `index.html`: try to resolve...
5. `index.plush.ko-kr.html` was *resolved by map* for `index.html`
3. `application.ko-kr.html`: try to resolve...
3. `application.ko.html`: try to resolve...
3. `application.en.html`: try to resolve...
2. `application.html` was localized as `application.html`
3. `application.html`: try to resolve...
5. `application.plush.html` was *resolved by map* for `application.html`
3. `_flash.html`: try to resolve...
5. `_flash.plush.html` was *resolved by map* for `_flash.html`

For the default language:

1. languages: [en en-US]
3. `index.en.html`: try to resolve...
2. `index.html` was localized as `index.html`
3. `index.html`: try to resolve...
5. `index.plush.ko-kr.html` was *resolved by map* for `index.html`
3. `application.en.html`: try to resolve...
2. `application.html` was localized as `application.html`
3. `application.html`: try to resolve...
5. `application.plush.html` was *resolved by map* for `application.html`
3. `_flash.html`: try to resolve...
5. `_flash.plush.html` was *resolved by map* for `_flash.html`

Debugging result of the updateAliases(): "which aliases are generated?"

   processing `index.plush.html` ...
     storing `index.plush.html` as `index.plush` (plush)
     storing `index.plush.html` as `index.html` (html`)
   processing `index.plush.ko-kr.html` ...
     storing `index.plush.ko-kr.html` as `index.plush` (plush)
     storing `index.plush.ko-kr.html` as `index.ko-kr (ko-kr)
     storing `index.plush.ko-kr.html` as `index.html` (html`)

As shown above, an unnecessary alias index.ko-kr is generated for the extension ko-kr, and the index.html and index.plush were overwritten by the last template every time and it became the default.

The first commit will fix this issue.

For the second issue, the calling time of resolve(), the original sequences are as follows:

when ko-KR is used for the request,

3. `index.plush.ko-kr.html`: try to resolve...
4. `index.plush.ko-kr.html` was **resolved directly** by the name
3. `index.plush.ko-kr.html`: try to resolve...
4. `index.plush.ko-kr.html` was **resolved directly** by the name

The template was evaluated twice. The second commit fixes this issue by introducing a new function localizedResolve(). The commit also improves the template-based i18n feature to support short language setting such as ko without the country code part, and also allows developers to create language-only templates such as index.plush.ko.html too.

@sio4 sio4 added the bug Something isn't working label May 5, 2022
@sio4 sio4 self-assigned this May 5, 2022
@sio4 sio4 requested a review from a team as a code owner May 5, 2022 11:53
@sio4
Copy link
Member Author

sio4 commented May 5, 2022

Fixed behavior for the first issue:

updateAliases()

   processing `index.plush.html` ...
     storing `index.plush.html` as `index.html`
   processing `index.plush.ko-kr.html` ...
     storing `index.plush.ko-kr.html` as `index.ko-kr.html`

searching

1. languages: [en en-US]
3. `index.en.html`: try to resolve...
2. `index.html` was localized as `index.html`
3. `index.html`: try to resolve...
5. `index.plush.html` was *resolved by map* for `index.html`
<...>

1. languages: [ko-KR ko en en-US en-US]
3. `index.ko-kr.html`: try to resolve...
5. `index.plush.ko-kr.html` was *resolved by map* for `index.ko-kr.html`
2. `index.html` was localized as `index.ko-kr.html`
3. `index.ko-kr.html`: try to resolve...
5. `index.plush.ko-kr.html` was *resolved by map* for `index.ko-kr.html`
<...>

Fixed behavior for the second issue:

calls only once:

3. `index.plush.ko-kr.html`: try to resolve...
4. `index.plush.ko-kr.html` was **resolved directly** by the name

also can use the short, language only locale

3. `index.ko.html`: try to resolve...
5. `index.plush.ko-kr.html` was *resolved by map* for `index.ko.html`

@sio4 sio4 marked this pull request as draft May 5, 2022 12:04
@sio4 sio4 changed the base branch from main to development May 7, 2022 15:00
@sio4 sio4 force-pushed the fix-i18n-support-for-templates branch from 8b81144 to 4b5b29e Compare May 14, 2022 15:36
@sio4 sio4 marked this pull request as ready for review May 14, 2022 15:46
@sio4 sio4 enabled auto-merge (rebase) May 14, 2022 15:47
@sio4
Copy link
Member Author

sio4 commented May 14, 2022

@paganotoni Please take a look at this PR too. It was blocked by #2262 but now works fine for all systems.

This fixes the template-based i18n issue which was the last language version of the template will be used as a template for the default language when the user specifies the template as index.html format instead index.plush.html. See details on OP.

@sio4 sio4 merged commit 1e4ac79 into gobuffalo:development May 15, 2022
@sio4 sio4 deleted the fix-i18n-support-for-templates branch May 15, 2022 07:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants