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

Avoid confusion between I, 1 and I in rendered URL #433

Closed
zej20 opened this issue Aug 19, 2020 · 14 comments · Fixed by #1317
Closed

Avoid confusion between I, 1 and I in rendered URL #433

zej20 opened this issue Aug 19, 2020 · 14 comments · Fixed by #1317
Labels
Hacktoberfest Issues for DigitalOcean's Hacktoberfest help wanted Extra attention is needed

Comments

@zej20
Copy link

zej20 commented Aug 19, 2020

I was recently sent a go.gov.sg url. The last character seemed to be either a 1 or a I. (I tried both but both failed. I guess there's another problem in there somewhere but let's deal with this first.) I think it is quite annoying for users to have to try both characters.

Could the code just avoid using "1" and "I" in the generated URL?

Granted, the poster I received had a QR code I could scan but it just adds additional complexity for the user in some cases. In my case, I was accessing the file from a computer and could not scan the QR code. Making the generated URL clickable in the QR code could be workable too I guess.

gov url

@LoneRifle
Copy link
Contributor

LoneRifle commented Aug 19, 2020

Hey @zej20 , and thanks for pointing out the usability problem! Given that you imply that you were reading the link and typing it in, would you think that using a different font to render the text would better aid users in accurately identifying the characters?

A clickable URL in the generated QR code does sound like a nice idea. One way we could try to facilitate this would be to offer embedded HTML to go along with the static images (which generally do not support embedding of clickable things which could potentially be dangerous). This proposed feature however would have to be balanced against making it relatively intuitive for users, who may not be familiar with the idea of copy-pasting HTML snippets into e-mails and would gravitate towards simple images.

Fwiw, I believe the character is the letter L in lowercase.

@zej20
Copy link
Author

zej20 commented Aug 19, 2020

Thank you, @LoneRifle ! It is indeed L in lowercase. I'd forgotten about that. Haha.

I am not sure about the complexity in the coding but I'd thought that avoiding the characters "1" (numeric one), "I" (uppercase I) and "l" (lowercase L) in randomly generated urls could be a straightforward solution. Those characters could still be kept as an option for custom urls requested by officers when they are contextually relevant as a word (eg: https://go.gov.sg/besafe-learn). Would this be possible? The alternative solution may not be needed if we side-step the problem I think.

A different font could be workable if there is font that can clearly differentiate between "1", "I" and "l" I guess.

@yong-jie
Copy link
Member

It looks like the code responsible for random generation is found over here. If we were to make this change it could be as straightforward as replacing lower-case 'l' to upper-case 'L' in ALPHABET etc.

const ALPHABET = '0123456789abcdefghijklmnopqrstuvwxyz'
const LENGTH = 6
/**
* Generate a random short URL.
* The chance of a collision with a 36-character alphabet
* of length 6 after 1 million URLs have been generated is
* (1e6 / (36^6) = 0.000459, or one in 2176.
*/
export async function generateShortUrl() {
return generate(ALPHABET, LENGTH)
}

@LoneRifle
Copy link
Contributor

I'm hesitant about that since it would violate the Principle of Least Surprise in some angles. I would lean towards using a font that helps users distinguish between the three characters

@liangyuanruo
Copy link
Contributor

it could be as straightforward as replacing lower-case 'l' to upper-case 'L' in ALPHABET etc.

No, that would not be backwards compatible with existing functionality, as we automatically lowercase all short links that visit the redirect URL.

@zej20
Copy link
Author

zej20 commented Aug 19, 2020

I agree that replacing lowercase L to uppercase L might potentially surprise the user if all the other characters used are lowercase. What if it is taken out entirely? This is what I am suggesting:

const ALPHABET = '023456789abcdefghijkmnopqrstuvwxyz'

I think this solution would not catch the user off guard since the user will not even encounter the characters that are hard to differentiate. But looking at the code, the associated issue would be the increase in the chance of a collision. I'm not sure how significant that might be.

Actually, having seen that code and realising that all the alphabets used are of lowercase, I think the problem is less significant now. But it is still potentially a problem as new users may not know that all the alphabets are of lowercase and may thus confused.

@LoneRifle
Copy link
Contributor

The Principle of Least Surprise also applies to engineers maintaining the codebase. My main concern is with implicit assumptions held by contributors as they go about making changes; the fewer the better.

Dropping 1, l and I might catch maintainers off-guard or at least add to the mental overhead whilst actively developing GoGovSG. My inclination for now will be to figure a way to better render the text in the image generated to users

@LoneRifle LoneRifle changed the title Prevent use of 1 and I in generated URL Avoid confusion between I, 1 and I in rendered URL Aug 19, 2020
@zej20
Copy link
Author

zej20 commented Aug 19, 2020

I see. Fair enough.

@liangyuanruo liangyuanruo added Hacktoberfest Issues for DigitalOcean's Hacktoberfest help wanted Extra attention is needed labels Aug 20, 2020
@liangyuanruo
Copy link
Contributor

TODO - Investigate using a serif font to render the URL.

@xming13
Copy link
Contributor

xming13 commented Aug 22, 2020

Some recommendation for the fonts to use to differentiate I (uppercase i) and l (lowercase L).
https://ux.stackexchange.com/a/79093

Preview:
https://fonts.google.com/?query=Sans&preview.text=IIIIlllliiii1111&preview.text_type=custom

@LoneRifle
Copy link
Contributor

Some recommendation for the fonts to use to differentiate I (uppercase i) and l (lowercase L).
https://ux.stackexchange.com/a/79093

Preview:
https://fonts.google.com/?query=Sans&preview.text=IIIIlllliiii1111&preview.text_type=custom

We will be limited to what we can find in ttf-freefont

@zanechua
Copy link

zanechua commented Oct 29, 2020

Font Styles in GNU FreeFont:

https://www.fontsc.com/font/designer/gnu?pt=IIIIlllliiii1111&ps=0&o=1&ls=10

image

IMO FreeMono would be a good choice.

This looks to be the code in question

.attr('font-family', 'sans-serif')

Would changing from sans-serif to monospace work?

@LoneRifle
Copy link
Contributor

I'm not keen on FreeMono to be honest, and gravitate towards using FreeSerif. @NatMaeTan - care to weigh in here?

@LoneRifle
Copy link
Contributor

LoneRifle commented Oct 30, 2020

We might want to swap ttf-truefont with font-ibm-plex-nerd. This should give us IBM Plex Sans, which is readable, and lines up with what we are already using in Go.

EDIT: No it doesn't - only IBM Plex is available

orbitalsqwib added a commit that referenced this issue Mar 17, 2021
- added appropriate styles to qr generation in `QRCodeService` to render IBM Plex Sans Regular
-
added wget command in Dockerfile to install IBM Plex Sans Regular in the container instance to give
sharp access to the font

fix #433
orbitalsqwib added a commit that referenced this issue Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest Issues for DigitalOcean's Hacktoberfest help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants