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

Unicode font uppercasing issues #3696

Merged
merged 13 commits into from
May 8, 2024

Conversation

obrusvit
Copy link
Contributor

@obrusvit obrusvit commented Apr 9, 2024

some_text.upper() calls do not work correctly in languages other than english. This PR adds additional Fonts which are UPPERCASE only. These fonts are used where appropriate, especially in Titles and Buttons.

  • For Model T, this meant only switching FONT_BOLD to FONT_BOLD_UPPER
  • For Model R, this meant adding additional fonts FONT_NORMAL_UPPER and FONT_BOLD_UPPER and use them where appropriate
  • For both models: some diffs emerged, listed below.

TODO:

  • do not duplicate glyphs for normal and _upper fonts
  • change other languages and it's fixtures
    • Spanish es
    • German de
    • French fr
    • Czech cs

@obrusvit obrusvit added core Trezor Core firmware. Runs on Trezor Model T and T2B1. T3T1 Trezor Safe 5 translations Put this label on a PR to run tests in all languages labels Apr 9, 2024
@obrusvit obrusvit self-assigned this Apr 9, 2024
@obrusvit obrusvit linked an issue Apr 9, 2024 that may be closed by this pull request
@Hannsek
Copy link
Contributor

Hannsek commented Apr 10, 2024

All summary screens use that. Can it be changed back?

@obrusvit
Copy link
Contributor Author

obrusvit commented Apr 15, 2024

Status (updated 2024/04/28)
Model T:

  • FIX: changing translation
    image

Model R:

  • FIX: PIN setting - the 2nd screen didn't have title and scrollbar. Moreover, if you went to 2nd screen and then back, the title was gone:
    2024-04-28_20-09_fix_PIN_settings_2
    and another case:
    2024-04-28_20-09_fix_PIN_settings

  • FIX: similar thing in Wipe code setting:
    2024-04-28_20-08_fix_wipe_code_setting

  • FIX: altcoin summary had title wrongly placed
    2024-04-28_19-55_fix_eth_altcoin_summary

  • FIX: changing translations
    2024-04-28_20-06_fix_change_lang_german

  • CHANGE from bold to normal in confirm message.
    2024-04-28_19-48_confirm_msg_size

  • CHANGE from lowercase to uppercase: all warnings are now with uppercased letter, some were not before. I believe these changes have no effect on the user. Full list:

    • receiving to multisig address
    • locktime has no effect
    • sending from multiple accounts
    • unusually high fee
    • many change outputs
    • unverified external inputs
    • group threshold reached
    • invalid seed/share entered
    • share already entered
    • share from different backup entered
    • wrong word selected (backup check)
    • PINs do not match
    • invalid PIN
    • wrong PIN
    • cardano - unknwon colateral
    • solana - unknown instructions
      Examples:
      2024-04-28_20-08_warning_wrong_pin
      2024-04-28_19-59_warning_invalid_seed_entered
      2024-04-28_19-49_warning_high_fee
  • CHANGE from uppercase to lowercase in "Please wait" screen
    2024-04-28_20-06_byg_please_wait

  • BUG: ETH Claim transaction is now lowercased on the 2nd screen
    2024-04-28_19-58_bug_eth_claim

@obrusvit obrusvit force-pushed the obrusvit/3600-unicode-font-uppercasing-issues branch from 39ac763 to 2bb40cd Compare April 17, 2024 10:39
@obrusvit obrusvit force-pushed the obrusvit/3600-unicode-font-uppercasing-issues branch from 2bb40cd to 754b701 Compare April 28, 2024 19:34
Copy link

github-actions bot commented Apr 28, 2024

legacy UI changes device test(screens) main(screens)

Copy link

github-actions bot commented Apr 28, 2024

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T2B1 Safe 3 3280 test(screens) main(screens) 2724
T3T1 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@obrusvit obrusvit force-pushed the obrusvit/3600-unicode-font-uppercasing-issues branch from 754b701 to b9075a1 Compare April 28, 2024 19:52
@obrusvit obrusvit added T2B1 Trezor Safe 3 (F4) T2T1 Trezor Model T labels Apr 28, 2024
@obrusvit obrusvit force-pushed the obrusvit/3600-unicode-font-uppercasing-issues branch from b9075a1 to 19a9533 Compare April 28, 2024 21:00
@obrusvit obrusvit marked this pull request as ready for review April 28, 2024 21:01
@obrusvit obrusvit requested review from mmilata and removed request for prusnak April 28, 2024 21:01
core/SConscript.firmware Outdated Show resolved Hide resolved
core/SConscript.bootloader Outdated Show resolved Hide resolved
@obrusvit obrusvit force-pushed the obrusvit/3600-unicode-font-uppercasing-issues branch 2 times, most recently from acbfe3d to 435babe Compare May 6, 2024 18:12
@obrusvit obrusvit force-pushed the obrusvit/3600-unicode-font-uppercasing-issues branch 2 times, most recently from d647341 to 9e239d8 Compare May 7, 2024 22:43
@obrusvit obrusvit requested a review from mmilata May 8, 2024 09:52
@obrusvit
Copy link
Contributor Author

obrusvit commented May 8, 2024

A lot of diff in fixtures due to many minor changes in languages as described in the previous comment.

Also some previous changes were not incorporated into fixtures from #3617

Some UI fixtures were missing for TS3, e.g. some tests of wipe code.

Copy link
Member

@mmilata mmilata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things but otherwise looking very good. T3T1 fixtures are missing.

},
"T2T1": {
"1_FONT_NORMAL": "font_tthoves_regular_21_cs.json",
"2_FONT_BOLD": "font_tthoves_bold_17_cs.json",
"2_FONT_BOLD": null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should swap 2<->7 for the scenario where newer translation blob is used on old firmware that expects existing font under id 2. (edit: but that would break compatibility on T2B1, never mind)

core/tools/codegen/gen_font.py Outdated Show resolved Hide resolved
core/tools/codegen/gen_font.py Outdated Show resolved Hide resolved
core/tools/codegen/gen_font.py Show resolved Hide resolved
core/embed/rust/src/ui/model_tt/theme/mod.rs Show resolved Hide resolved
@obrusvit
Copy link
Contributor Author

obrusvit commented May 8, 2024

T3T1 fixtures also updated @mmilata f53f8ab

Copy link
Member

@mmilata mmilata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, feel free to rebase and merge if CI passes.

obrusvit added 13 commits May 8, 2024 18:27
The new option allows generation of font glyph definition where both
lowercase and uppercase letters are mapped to uppercase glyphs.

[no changelog]
By default, buttons have uppercased font. The addition is necessary to
customize font on info buttons where we need normal font.

[no changelog]
The title previously disappeared after going back and forth. Used in PIN
setting and FIDO confirm.

[no changelog]
The change is necessary for future models where titles might not be
uppercased.

[no changelog]
Updating after UTF-8 uppercasing issue.

Only changes for Model T are during language change where it actually
fixes a problem.

Model R has more changes - especially in show_warning invocation.

See PR 3696 for more info.

[no changelog]
Probably an omission after pull #3710.

Update using
../tests/update_fixtures.py local -r

[no changelog]
This commit changes translations strings of ES, FR, CS, DE langs so that
they do not use UPPERCASED titles, button labels and other texts.

[no changelog]
@obrusvit obrusvit force-pushed the obrusvit/3600-unicode-font-uppercasing-issues branch from f53f8ab to cf5ac3f Compare May 8, 2024 16:30
@obrusvit obrusvit merged commit 30b50c6 into main May 8, 2024
107 of 109 checks passed
@obrusvit obrusvit deleted the obrusvit/3600-unicode-font-uppercasing-issues branch May 8, 2024 20:46
@bosomt
Copy link

bosomt commented Jul 1, 2024

QA OK

tested on Czech translation

  • Device: Trezor T2T1 2.7.2 regular (revision da75d8f)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Trezor Core firmware. Runs on Trezor Model T and T2B1. T2B1 Trezor Safe 3 (F4) T2T1 Trezor Model T T3T1 Trezor Safe 5 translations Put this label on a PR to run tests in all languages
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Unicode font uppercasing issues
5 participants