Translation typos, weblate problems, UI remarks & improvments submission

ysard commented 1 year ago

Hello, thank you for this application which is more and more useful these days, including in France where the Internet is under more and more pressure from sick governments. I am currently finishing the FR translation and I would like to share with you some improvements/bugs that I have noticed during this work and the use of the app. Please let me know if I need to open several issues.

[x] String dns_firewall_mode_info_explanation_new https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L159 This wrongly implies that in addition to DNS and Firewall the app adds censorship workarounds. So I translated it as "All of the above for optimal Internet censorship bypass."
[ ] Distinguishing the level of application of the firewall (appli vs universal) in request filters is not easy.

Currently it is partially done for strings like "firewall_rule_lockdown" and "firewall_rule_global_lockdown": https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L931 https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L946

I suggest you add "(App)" and "(Univ)" or "(Universal)" in each case.

Also, you propose:
- 2 status for metered (app and universal): firewall_rule_block_metered, firewall_rule_univ_block_metered
- only 1 status for unmetered (app): firewall_rule_block_unmetered
  
  https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/java/com/celzero/bravedns/service/FirewallRuleset.kt#L32-L37 => the universal one is maybe forgotten
Also, it is impossible to distinguish (other than visually via the icon, which is not obvious) between the 2 types of rules "IP / Port". Indeed, the same string firewall_rule_bypass_universal_ip is used for both: https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/java/com/celzero/bravedns/service/FirewallRuleset.kt#L40-L46

Could you make this distinction easier?
[x] Some strings are not translated: https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/java/com/celzero/bravedns/util/Utilities.kt#L229-L234
[ ] String arrays are not supported by weblate? These elements are not available for translation: https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L45-L52
[x] At least one debug text is made available for translation. I think this is a mistake that will not simplify your work of analyzing the logcats. In addition, in this same place I noticed a significant duplication of code that could cause you problems in the future: https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/java/com/celzero/bravedns/adapter/ConnectionTrackerAdapter.kt#L96-L98 https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/java/com/celzero/bravedns/adapter/AppIpRulesAdapter.kt#L78-L81 https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/java/com/celzero/bravedns/adapter/AppConnectionAdapter.kt#L87-L90
[ ] Could you enable the strict-same flag on invariant terms in the app to avoid false warnings like "translation unchanged" on weblate? See: https://docs.weblate.org/fr/latest/user/checks.html#check-same Examples of strings I encountered during my translation:
```
RethinkDNS
RethinkDNS Plus
RDNS Plus
DNS over HTTPS
DNSCrypt
vpn
Orbot
DoH
+ urls in general
```
[ ] You have a string duplication on these strings. I think the slight variation between the 2 is not intended. It would be easier to combine them into one for easier maintenance & translation work: https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/layout/fragment_about.xml#L73 https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L578

https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/layout/welcome_slide2.xml#L33 https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L28
[x] Remarks on the use of some strings. I don't know where these strings are used in the application. So I can't translate them correctly.

https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/java/com/celzero/bravedns/automaton/FirewallManager.kt#L69-L90

https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L55-L60
[x] About the setting in settings_https_desc: https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L326 Is it possible to make applications ignore this parameter? Or is it just that no matter how much you specify it, some apps won't take it into account anyway?
[x] About the settings_lock_down_mode_desc: https://github.com/celzero/rethink-app/blob/ed8616ba94e757e62ba634a1f65d192deeaed708/app/src/main/res/values/strings.xml#L318-L320 I don't understand when it triggers.

Probably when you try to allow bypassing of some applications with the "Allow Bypass" option in the settings? What does VPN in containment mode mean? I don't see an option to do that.
[x] There is a typo here:

The system seems to add "Il y a" in French, which is redundant with the previous word "Since".

[x] UI & access to DNS and firewall functions from the home screen:

DNS:
- impossible when DNS protection is not activated (request to activate it: "you are at one step...")
- impossible when the protection is active and a connection is active (mobile or wifi) ("Private DNS has been set")
- possible only when DNS protection is active but there is no connection in progress
Firewall:
- possible when the firewall protection is active but there is no connection in progress
- possible when firewall protection is active and there is a connection in progress
- impossible if the firewall protection is inactive
For the sake of consistency we would like to see the DNS logs with an active connection.
[x] UI & access to tabs: DNS: config tab is on the right, logs are on the left Firewall: config tab on the left, logs on the right

The order of the tabs should be the same.

ignoramous commented 1 year ago

Hi, thank you for your review. It must have taken you a lot of time to do it! Appreciate your inputs.

This wrongly implies that in addition to DNS and Firewall the app adds censorship workarounds.

No, it isn't incorrect. It does apply censorship bypass by splitting the SNI field into two which is pretty effective at tripping up rudimentary censors like those found in multiple countries.

Re: Raw Strings in code: That's just lousy from us. We'll add it to strings.xml.

Re: Other issues: @hussainmohd-a is the lead developer, and I'll inform him to take a look.

ysard commented 1 year ago

Thank you for your your quick and encouraging response.

It does apply censorship bypass by splitting the SNI field into two which is pretty effective at tripping up rudimentary censors like those found in multiple countries.

Ok, I keep the original translation about that. I think you should communicate more about this useful feature because I didn't see any sign of it in the application (as an option for example). So it is only active with the DNS+Firewall combo?

ignoramous commented 1 year ago

...communicate more about this useful feature

It is the default mode for a reason. And when you change the mode, you can't really miss look at the label... (:

So it is only active with the DNS+Firewall combo?

Technically, this is also available with just the Firewall, but pretty effective with DNS + Firewall.