Client: Fix wrong truncation of unicode aliases#2275
Client: Fix wrong truncation of unicode aliases#2275hoffie merged 1 commit intojamulussoftware:masterfrom
Conversation
Did you mean "inline" rather than "include"? As a minor thing, I'm not a fan of non-trivial code being in a |
What I meant is that without moving the file to the (included)
I agree, but I initially tried to keep the code as-unmodified as possible. Keeping the So, I've dropped both |
If the 16th char of the user-selected alias was a multi-byte unicode char, it would be incorrectly split in half, rendering it invalid. This commit makes the truncation unicode-aware. The fix was initially developed in ce370e9. It got lost during PR merge in commit a8853b6, most likely during conflict resolution due to code reorganizations in d94fd69. This commit re-applies the fix to the new place (CClientSettingsDlg::OnAliasTextChanged instead of the no longer existing CMusProfDlg::OnAliasTextChanged). It also adds a function declaration to the header to make inclusion by other files possible in the first place. This also required dropping the `static` and `inline` keywords. Fixes jamulussoftware#1994 (for real). Fixes jamulussoftware#2274. Co-authored-by: Martin Kaistra <[email protected]>
softins
left a comment
softins
left a comment
There was a problem hiding this comment.
I was unable to demonstrate the problem in a version prior to this fix, so couldn't do a before and after comparison. But the code looks fine, and alias entry and display works correctly with an accented character at the end,so happy to approve.
|
the original issue mentions the test string " |
Yes, that was the one that showed it up when trying to line-wrap the name on the mixer display, and was already fixed. But this PR is addressing truncation after 16 characters in the settings window, which I couldn't make go wrong in a prior version, with a multi-byte char at the end. |
Try putting this string in the settings window alias field and then insert characters before the multi-byte character until you get a question mark character at the end. |
Sorry, should have documented my test case as it also took me some time to understand and reproduce the problem. Before this fix: |
4 participants
Short description of changes
If the 16th char of the user-selected alias was a multi-byte unicode char, it would be incorrectly split in half, rendering it invalid. This commit makes the truncation unicode-aware.
The fix was initially developed in ce370e9. It got lost during PR merge in commit a8853b6, most likely during conflict resolution due to code reorganizations in d94fd69.
This commit re-applies the fix to the new place (
CClientSettingsDlg::OnAliasTextChangedinstead of the no longerexisting
CMusProfDlg::OnAliasTextChanged). It also adds a function declaration to the header to make inclusion by other files possible in the first place. This also required dropping thestaticandinlinekeywords.Co-authored-by: @djfun
See #2274 for more context.
Context: Fixes an issue?
Fixes: #1994 (for real)
Fixes: #2274
Does this change need documentation? What needs to be documented and how?
No. ChangeLog has already been slightly adjusted to include this issue number.
CHANGELOG: SKIP
Status of this Pull Request
Ready, should go into 3.8.2.
What is missing until this pull request can be merged?
Needs review.
Checklist