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

Bugfix: quote monospace font #85

Merged
merged 2 commits into from Oct 7, 2021
Merged

Conversation

tusharsadhwani
Copy link
Contributor

@tusharsadhwani tusharsadhwani commented Oct 7, 2021

For some reason, saying just monospace here doesn't seem to work on chrome mobile. Quoting monospace fixes the issue.

Before:

After:

For some reason, saying just `monospace` here doesn't seem to work on chrome mobile. Quoting monospace fixes the issue.
@nedbat
Copy link
Member

nedbat commented Oct 7, 2021

See https://twitter.com/sadhlife/status/1446060335979397120 for the sleuthing :)

@hugovk
Copy link
Member

hugovk commented Oct 7, 2021

Good work! As it happens, this is fixed in the upcoming Chrome 96: python/pythondotorg#1708 (comment)

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Despite the upcoming fix in Chrome, this is such a simple fix, I think it's worth merging for people who will still be on Chrome < 96, even after Chrome 96 release.

(Plus for people using latest Chrome 94 now :)

@JulienPalard
Copy link
Member

Oh this kind of bugs … ☹

@JulienPalard JulienPalard merged commit 60d013d into python:master Oct 7, 2021
@JulienPalard
Copy link
Member

Thanks @tusharsadhwani and @nedbat!

@JulienPalard
Copy link
Member

I released it as python-docs-theme 2021-11 (I live in the future) and rebuilt https://docs.python.org/3/ so someone can check. Rebuild for other versions and languages will be automatically done in the next 24h.

@beauremus
Copy link

This "fix" broke the font for me. My Chrome on my Mac is now loading Helvetica for code rather than Courier. I can confirm that removing the quotes from monospace fixes the issue for me. I recommend this change be reverted and wait for a Chrome mobile fix. Let me know if I need to create a PR.

@tusharsadhwani
Copy link
Contributor Author

Chrome, showing 3 different behaviours on 3 operating systems. A tale as old as time.

@nedbat
Copy link
Member

nedbat commented Oct 7, 2021

Perhaps the best fix is to just specify monospace. Why a fallback to sans-serif? Does any browser not have a monospace font that is used when you specify monospace?

@tusharsadhwani
Copy link
Contributor Author

Removing the quotes from monospace even with it being the only specified font breaks on current chrome mobile.

@hugovk
Copy link
Member

hugovk commented Oct 7, 2021

So... Do we need one monospace with quotes and one monospace without quotes? 🙃

Perhaps the best fix is to just specify monospace. Why a fallback to sans-serif? Does any browser not have a monospace font that is used when you specify monospace?

The problem seems to be desktop isn't picking up the "monospace" and falling back to sans-serif. If we remove sans-serif, it'll fall back to some system default, which will be some sans-serif or serif, but not monospace.

@tusharsadhwani
Copy link
Contributor Author

Okay, I tested on 3.9's page, and it seems just

pre {
  font-family: monospace;
}

seems to work. Can someone test it on all 3 platforms, android mac and windows, that'd be great.

@tusharsadhwani
Copy link
Contributor Author

Android, Chrome 94

Linux, Chrome 94
image

@arseniiv
Copy link

arseniiv commented Oct 7, 2021

Noticed my Firefox 93.0 on win10 is weirded by "monospace" and falls back to sans-serif, though it underlines "monospace" in the style inspector as if it was used. Using font-family: "monospace", monospace, sans-serif; seems to work fine, though. Would this variant satisfy Chrome?

@brandon-leapyear
Copy link

brandon-leapyear commented Oct 7, 2021

This is an old work account. Please reference @brandonchinn178 for all future communication


Can confirm, currently fonts on docs.python.org are NOT monospaced on Chrome. font-family: "monospace", monospace, sans-serif does work.

@tusharsadhwani
Copy link
Contributor Author

tusharsadhwani commented Oct 7, 2021

font-family: "monospace", monospace, sans-serif;

Can confirm, works fine. Even on mobile where just monospace, sans-serif doesn't.

@hugovk
Copy link
Member

hugovk commented Oct 7, 2021

There is (or was) a known problem with plain font-family: monospace; that can cause text resizing issues, see necolas/normalize.css#519 (comment). The workaround is font-family: monospace, monospace; or
font-family: monospace, anyotherfont;`

And necolas/normalize.css#519 (comment) suggests a reason, browsers looked for the fixed string font-family: monospace;

@tusharsadhwani
Copy link
Contributor Author

Ah, that makes sense now. So I guess monospace, "monospace"; it is then :P

@beauremus
Copy link

beauremus commented Oct 7, 2021

MacOS Chrome 94 monospace
Screen Shot 2021-10-07 at 12 34 04 PM
MacOS Chrome 94 monspace, "monospace"
Screen Shot 2021-10-07 at 12 37 52 PM

Both solutions work for me.

@tusharsadhwani
Copy link
Contributor Author

@beauremus Thanks a bunch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants