Use client side translation for public login page

[Chocobo1]

The translation strings are meant to be synced from Transifex.

[sledgehammer999]

Having to specify an id for each translatable element to grab it via JS, then list all translatable strings in one place in the JS and keep that list in sync with the correct element id will become tedious fast. It is also error prone.

I propose an alternative approach. I didn't find exact documentation for this in i18next itself, but I gathered enough information from examples in stackoverflow and some hints from the i18next-parser.

Take for example the login form:

<form id="loginform">
    <div class="row">
        <label for="username" id="usernameLabel">Username</label><br />
        <input type="text" id="username" name="username" autocomplete="username" required />
    </div>
    <div class="row">
        <label for="password" id="passwordLabel">Password</label><br />
        <input type="password" id="password" name="password" autocomplete="current-password" required />
    </div>
    <div class="row">
        <input type="submit" id="loginButton" value="Login" />
    </div>
</form>

Let's drop the id , add the strings via a data-* attribute and mark each translatable element with a class translatable like so:

<form id="loginform">
    <div class="row">
        <label for="username" data-i18n="Username" class="translatable">Username</label><br />
        <input type="text" id="username" name="username" autocomplete="username" required />
    </div>
    <div class="row">
        <label for="password" data-i18n="Password" class="translatable">Password</label><br />
        <input type="password" id="password" name="password" autocomplete="current-password" required />
    </div>
    <div class="row">
        <input type="submit" data-i18n="Login" class="translatable" value="Login" />
    </div>
</form>

Now on the JS side, after the translations are fetched you assign the strings with something like this:

const elements = document.getElementsByClassName("translatable");
  for (let i = 0; i < elements.length; i++) {
    const element = elements[i];
    const k = element.getAttribute("data-i18n");
    element.innerHTML = i18next.t(k);
  }

String extraction into the .json files

If you read the docs of i18next-parser further down where they talk about the options each lexer takes, you'll see that the HTMLLexer supports the data-i18n attribute by default and you can add more attributes if you like. So it scan that attribute to extract the "key" (aka the translatable string).

What do you think?

[glassez]

@Chocobo1
Is it the intention to apply this translation method to the entire WebUI?

[Chocobo1]

Is it the intention to apply this translation method to the entire WebUI?

Only to the login page. It is written in the PR title.

For now, I have not considered applying to the whole WebUI and I'm not sure how that will turn out.

[glassez]

Is it the intention to apply this translation method to the entire WebUI?

Only to the login page. It is written in the PR title.

For now, I have not considered applying to the whole WebUI and I'm not sure how that will turn out.

Is it acceptable to use different translation methods at the same time?

[Chocobo1]

Is it acceptable to use different translation methods at the same time?

It is ok but hopefully it can transition to only one method eventually.

[glassez]

Is it acceptable to use different translation methods at the same time?

It is ok

Won't this complicate the job of translators?

but hopefully it can transition to only one method eventually.

Wouldn't it make sense to provide instructions on how to switch the source code to another translation method so that it would be easier for someone else to contribute to such a gradual transition?

[Chocobo1]

PR updated. Adopted suggestion from #20520 (comment)

Won't this complicate the job of translators?

There will be 2 sources of translation strings of course. If you are talking about this PR adding another source then I'm not concerned of it. It has only 5 strings and the login page probably won't see many changes in the future.

Wouldn't it make sense to provide instructions on how to switch the source code to another translation method so that it would be easier for someone else to contribute to such a gradual transition?

Would a few basic instructions help? I don't know, it still sounds like a monumental task. There are probably some edge cases and requires specific handling to make it work.

[glassez]

So, do we need to add each file here separately later?

[Chocobo1]

It might support wildcards though.
Also it would be better to append another command to this script: ... && i18next -c private/i18next-parser.config.mjs private/*.html ...

[sledgehammer999]

Yes, it supports globbing patterns as evidenced in their readme examples: https://github.com/i18next/i18next-parser

[glassez]

Would a few basic instructions help? I don't know, it still sounds like a monumental task. There are probably some edge cases and requires specific handling to make it work.

Or maybe add all the necessary scripts and settings to allow translate files from private folder?

[Chocobo1]

Or maybe add all the necessary scripts and settings to allow translate files from private folder?

You are really interested? I can try lay down the foundations for private folder and leave the string migrations to others. (In another PR of course)

[glassez]

Or maybe add all the necessary scripts and settings to allow translate files from private folder?

You are really interested? I can try lay down the foundations for private folder and leave the string migrations to others.

I would just like to "open the doors wider" for other contributors to speed up this process as much as possible.
My motivation is as follows:
I believe that we disabled the applying the translations to alternative WebUIs thoughtlessly, thereby creating a problem for the use of so-called WebUI themes (this is when someone wants to use the original WebUI with minor modifications, mostly visual). Switching the WebUI to client-side translations would automatically save us from this problem. However, if this cannot be done in the near future, I would insist on re-enabling the apllying translations to alternative WebUIs in the upcoming 5.0 version and maybe even in the current v4.6.x branch.

[Chocobo1]

However, if this cannot be done in the near future, I would insist on re-enabling the apllying translations to alternative WebUIs in the upcoming 5.0 version and maybe even in the current v4.6.x branch.

I would prefer there be an option to enable this behavior. (and off by default)

[sledgehammer999]

IMO there is no point. Feeding more than what i18next needs doesn't improve anything. From my testing it will only use the major language and only when it doesn't exist then the fallback lang will be used.

I am not sure currently how we choose the language to display.
In other websites, the user is able to choose a different language on the fly by clicking a menu item. For that particular case, it makes sense to load everything into i18next and just change the lgn key when a user chooses another language. But we probably don't implement that now, so it probably makes sense to load only the specific language.

[Chocobo1]

I am not sure currently how we choose the language to display.

This PR currently detects the preferred language via navigator.languages (run it in your browser console to see the lang list). And then the client tries to fetch all possible matches from the server and present the closest/best match.

In other websites, the user is able to choose a different language on the fly by clicking a menu item. For that particular case, it makes sense to load everything into i18next and just change the lgn key when a user chooses another language. But we probably don't implement that now, so it probably makes sense to load only the specific language.

IMO it would be OK to load the translation resource on demand in this case. I.e user select a language then the client tries to load it instead of loading all translation resources at once at start.
But I imagine this would require more steps when bundling the resources (to build the available lang list) which I'm clueless yet. I would say let's leave the investigation for a later PR.

[sledgehammer999]

@Chocobo1 I pushed two commits into your branch, because it was faster for me to implement it than try to explain it here/request it.

The first commit implements a script that will migrate translated strings from the .ts files to the .json files.
It would be wrong to request translators to redo all of their work. This tool can help with the gradual migration of the rest of the WebUI. It updates untranslated .json strings if the translation exists in the corresponding .ts file.

The second commit has pulled the translated strings as they existed before the merging of f265eb0.

If you like, you can squash the second commit into your own. However, I would like my first commit to be as-is unless you want to work on the code/name/whatever more.

[sledgehammer999]

This PR currently detects the preferred language via navigator.languages (run it in your browser console to see the lang list). And then the client tries to fetch all possible matches from the server and present the closest/best match.

@glassez I was wrong. This code doesn't request every single language file. It requests the user's locales (and it's variations) plus the english locale. eg [el-GR, el, en]
So, it is ok as it is.

[sledgehammer999]

My final objection is in regards of the .tx/config file:
Do we really need 2 separate translations resources for the WebUI in Transifex? One for the public part and one for the private.
Can't we workaround this and have only one WebUI resource in Transifex?
Somehow split/merge it locally for the different parts? Or simply duplicate the .json files between the two parts?

[sledgehammer999]

This code doesn't request every single language file. It requests the user's locales (and it's variations) plus the english locale.

@Chocobo1 as an optimization I don't think you need to ever load the en JSON, because .t() will fallback to the key anyway.

[Chocobo1]

Can't we workaround this and have only one WebUI resource in Transifex? Somehow split/merge it locally for the different parts?

I honestly don't have better ideas. Do you really insist on it? The public page only has 5 strings so far and is very unlikely to be more in the future. The public login page is intentionally kept dead simple.

Or simply duplicate the .json files between the two parts?

Do you mean exporting json of the private folder and use it for public folder? But won't the translation file become unnecessarily big for the public folder?

[Chocobo1]

@Chocobo1 as an optimization I don't think you need to ever load the en JSON, because .t() will fallback to the key anyway.

I assumed the navigator.languages might be empty and when it is, the lng parameter for i18next will be undefined. I think that would be weird and decided to still provide en as a fallback to make it look consistent.

[glassez]

I honestly don't have better ideas. Do you really insist on it? The public page only has 5 strings so far and is very unlikely to be more in the future. The public login page is intentionally kept dead simple.

IMO, the general question is should we keep maintaining this separation of assets of private and public? The really private data is provided via API only.

[sledgehammer999]

I honestly don't have better ideas. Do you really insist on it? The public page only has 5 strings so far and is very unlikely to be more in the future. The public login page is intentionally kept dead simple.

I have slept on it and I propose the following workflow.
Have these 3 locations:

1. public/lang/<LOCALE>.json
2. private/lang/<LOCALE>.json
3. transifex/en.json

The 2 first will contain the generated .json files for all locales for private/public part. Via a script the en.json for each part will be merge into one JSON and put under transifex/. This will serve as the source file for the relevant single Transifex resource.
Now, in reverse, when translations are pulled from Transifex they will be written under transifex/. These files are temporary and won't be commited as-is in git. For each locale, a script will load the temporary file and update the corresponding public/private JSON file. This should be easy/simple because it is basically a key lookup in a dictionary.

[Chocobo1]

IMO, the general question is should we keep maintaining this separation of assets of private and public?

I consider giving free access for any assets is a bad idea generally, even if they are insignificant.

I have slept on it and I propose the following workflow. Have these 3 locations:

1. `public/lang/<LOCALE>.json`

2. `private/lang/<LOCALE>.json`

3. `transifex/en.json`

Looks doable to me. I'm OK If you are OK with the additional steps when syncing translations.

[sledgehammer999]

I implemented the new tool and squashed it into my previous commit. Then I force pushed to your branch (preserving your new commit) but the PR hasn't updated.

[sledgehammer999]

I implemented the new tool and squashed it into my previous commit. Then I force pushed to your branch (preserving your new commit) but the PR hasn't updated.

If I somehow broke Github, then you are free to drop my commits and resubmit your own changes (and address the rest of the review comments). Then I'll reintroduce my commits in a new PR.

[Chocobo1]

If I somehow broke Github, then you are free to drop my commits and resubmit your own changes (and address the rest of the review comments). Then I'll reintroduce my commits in a new PR.

It looks OK to me. I can see the 2 python scripts.

[sledgehammer999]

@Chocobo1 let me explain my intended workflow which makes the CI change unneeded.

On some interval, I'll pull the translations from Transifex. These will be dropped into transifex/*.json.
Then I'll call split_merge_json.py --mode split which will split transifex/*.json into their equivalent public/lang/*.json and private/lang/*.json parts. The non english transifex/*.json files won't be commited to git.
Afterwards I'll run the i18next-parser commands to extract possible new strings into public/lang/*.json and private/lang/*.json. These will be committed to git.
Finally, I'll run split_merge_json.py --mode merge which will merge public/lang/en.json and private/lang/en.json into transifex/en.json. This will be committed to git.
Transifex will be setup to regularly pull transifex/en.json from the repo.

qbittorrent_webui_json

isn't it a new resource on transifex? Shouldn't I remove this whole section instead?

source_file = src/webui/www/transifex/en.json

Do I also need to change the path that the i18next-parser emits? (in the config file i18next-parser.config.mjs)

So now we need just one resource on Transifex. Since the old WebUI translations will still be active we can't reuse the same name for the resource.

[sledgehammer999]

let me explain my intended workflow which makes the CI change unneeded.

And to add to the above:
When the incremental transition will be made in the private/ part the author shouldn't generate any *.json files. Just act on the html/js files. The files will be generated by my above workflow. At the appropriate step of my workflow I'll insert a call to translations_qt2i18next.py to migrate the strings.

[Chocobo1]

@sledgehammer999
I've updated the PR and should addressed all remaining comments.
I also added sort_keys=True to json.dump() in the new scripts. This is to make sure the output is always sorted and stable.

[sledgehammer999]

I'll probably merge this in the next hours so it can be included in the unstable build.