PreCreateWallet: Replace more name chars. #3511
Conversation
|
It might also be better to change to only allow a certain set, which may be more complete. I think some windows systems in different languages have other special characters. |
Disallow some problematic characters in wallet names.
|
@degeri Any opinion on what to allow or disallow? |
|
Cant we go for a whitelist ? a-z,A-Z,0-9 |
This would be too restrictive for non-US speakers (e.g. can't name a wallet ¨Poupança"). |
Yes it would be. The problem is that if we extend to many languages we will hit alot of problems like R->L chars, Long chars ด้้้้้็็็็็้้้้้็็็็็้้้้้้้้็็็็็้้้้้็็็็็้้้้้้้้็็็็็้้้้้็็็็็้้้้้้้้็็็็็้้้้้็็็็ etc. Not sure how we handle that currently tbh. Maybe we should test with https://github.com/minimaxir/big-list-of-naughty-strings/blob/master/blns.json and see which cause errors/inconsistencies . |
|
After much discussion @matheusd and me came to the conclusion that denying any unicode char (Unless it specifically causes and issue) will be bad UX for users. Even the bug due to |
|
To expand a bit on degeri's comment: Users should be able to (at least in principle) use any wallet name they so choose. However, certain special chars need to be explicitly filtered due to us directly using the wallet name as part of the path to the wallet's files. The list of chars that I believe need to be filtered out are:
|
|
Blacklisting suggested chars. Thank you. |
| if (newWalletName.length > 0 && newWalletName[0] === " ") | ||
| newWalletName = newWalletName.slice(1); |
There was a problem hiding this comment.
You could have multiple spaces, so best to just use trim()
There was a problem hiding this comment.
How do you mean. It runs this code every key press, so trim disallows any spaces at all. I could put trim in somewhere else I suppose?
There was a problem hiding this comment.
Should we also disallow spaces? I just thought a leading space was silly and confusing, but something like "My Wallet" should be fine.
closes #3359
This works in the same way it did before, just adds a few more characters to the replace and doesn't allow for leading spaces. space, +, -, _, and : look to be fine on linux, but Windows may be more restrictive. Will test there if noone else can. Parenthesis and brackets may also be fine and used by some people. Allow?