Code review comment for lp:~jonas-drange/ubuntu-system-settings/security-pass-input-fix-1357548

Revision history for this message
Matthew Paul Thomas (mpt) wrote :

I'm delighted that this confusing bug is being fixed! I am not an engineer, and I might be greatly misunderstanding what is going on ... But this approach does not look right to me.

Pat wrote in bug 1357548 that "We currently set an inputMask of '9999' which causes the pre-population of asterisks". From what I can tell, you have then brainstormed a way to achieve the same effect as an inputMask, without using inputMask. Correct?

If so, this has two problems. First, the code is longer.

Second, if any third-party developer writes an app containing a password field, where the password characters need to be restricted in any way, they might reasonably read <https://developer.ubuntu.com/api/qml/sdk-14.10/Ubuntu.Components.TextField/#inputMask-prop> and think that using an inputMask is a good way to achieve that. But if they do, they will have exactly the same bug in their app: weird bullet characters in an empty field. Right? It will be a booby-trap in the toolkit.

Either inputMask is a good idea or it isn't. If it is, ubuntu-system-settings should continue using it, and this bug should be fixed in the toolkit. And if it isn't, it shouldn't be in the toolkit at all.

review: Needs Information

« Back to merge proposal