jupyter / notebook

Jupyter Interactive Notebook
https://jupyter-notebook.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
11.52k stars 4.82k forks source link

Safari monospace font issue #4517

Open goranmoomin opened 5 years ago

goranmoomin commented 5 years ago

I can't understand why this is happening, but on Safari 12.0.3 in macOS High Sierra 10.13.6, fonts are not monospaced.

non-monospace font

Could also reproduce same issue in macOS Mojave 10.14.3 with Safari 12.0.3.

Strangely, when I change the CSS code of .CodeMirror from font-family: monospace to font-family: [any monospace font name here] it displays well.

E.g. something like this

.CodeMirror {
-   font-family: monospace;
+   font-family: SF Mono;
    height: 300px;
    color: black;
    direction: ltr;
}

I'm not a CSS guru, but this issue doesn't happen in plain CodeMirror editor. Maybe Jupyter notebook's CSS is overriding the default monospace font? (Is this even possible?)

minrk commented 5 years ago

I don't believe we can override the default monospace font. This should be browser configuration. What do you see on this page?

goranmoomin commented 5 years ago

@minrk That works fine... It's using Courier New. As I said, the plain CodeMirror editor also works fine, so I'm not sure what's happening. While I was searching for this, I found this SO question, but there is no answer yet :-(

zcqian commented 5 years ago

I am seeing the exact same issue here. Perhaps it is related to user's locale settings? I will test this if it would be helpful.

I can confirm that on Firefox, text areas in notebooks use monospace fonts correctly.

goranmoomin commented 5 years ago

Well, I found that when I delete the attribute lang of tag html in DevTools, monospace fonts come back. So, something like this:

- <html lang="ko-kr">
+ <html>

makes from this:

무제

to this:

무제

I tested this only in BinderHub (not in local), but I'm fairly sure this is the problem.

Is there any :lang() selectors in the CSS that is overriding fonts?̊̈ I'm searching through the DOM and CSS files, but I'm having no luck :-(

adah1972 commented 4 years ago

I am having this issue. My temporary workaround is go with Notebook 5.4.1: only Notebook 6 has this problem.

goranmoomin commented 4 years ago

@adah1972 Hmm, for me this specific issue is resolved in macOS 10.15 Catalina; did you upgrade?

Sent with GitHawk

goranmoomin commented 4 years ago

For anyone who’s interested in, this ‘bug’ is (or was) a Safari bug, not a jupyter notebook one. When the top level html element has a special type of lang attribute, the default monospace font (once) changed to a non-mono one in Safari.

By executing the following JS code line by line in the JS console inabout://blank, the bug would be more apparent:

document.body.append(document.createElement("pre");
document.querySelector("pre").innerText = `#include <stdio.h>
int main(void) {
    printf("Hello, World!\\n");
    return 0;
}`;
document.querySelector("html").setAttribute("lang", "ko");

The monospaced code‘s font will change after setting the lang attribute.

Sent with GitHawk

adah1972 commented 4 years ago

Thanks, pcr910303.

To maintainers of Notebook:

Any possibility to add a workaround for lower versions of macOS? After all, Notebook 5 is working well. Also, I do not understand why a lang attribute should be added. Maybe the easiest approach is not add the lang attribute unless you know it is safe to do so?

I do not think everyone is upgrading. I do not plan to do so in the near future.

goranmoomin commented 4 years ago

@adah1972 I’m not sure if the fix is backported for lower versions of macOS, but try checking if a Safari update is available.

Also, to give a tip; I had a bookmark that pointed to: javascript: querySelectorAll%28%22html%22%29.removeAttribute%28%22lang%22%29. Every time you see non-monospace fonts in Jupiter notebooks, just click the bookmark and your ready to go!

(You should visit any website, add to bookmarks and edit the bookmark’s URL to the desired one to add a javascript: URL)

Sent with GitHawk

adah1972 commented 4 years ago

@pcr910303 Thanks for the hint, but that would be a big burden. It seems the more viable options are:

  1. Stick with Notebook 5
  2. Use Firefox/Chrome instead
  3. Wait until the maintainers has time to help us Mac users…
goranmoomin commented 4 years ago

@adah1972 After some investigation, looks like here is the offending line; I would recommend opening a PR removing that line.

I'm too lazy (and this isn't an issue for me) to clone, check and open a PR.

adah1972 commented 4 years ago

@pcr910303 I can understand this issue, but I am no JavaScript developer, and cannot provide a sound, tested patch.... 😂

@ashleytqy I noticed you introduced the change on this line, so I think you are familiar with the related logic. Any possibility to add a workaround for macOS users lower than 10.15?

My Safari version is 13.0.2. @pcr910303 Your version?

goranmoomin commented 4 years ago

@adah1972 Well, the only change you should make is to remove that line, and as far as I checked, nothing will be affected. No need for additional changes.

Testing won’t need any JavaScript knowledge, it’s just installing the python package locally and executing it.

My Safari version is also 13.0.2; the fix apparently hasn’t been backported :-(

Sent with GitHawk

adah1972 commented 4 years ago

Simply removing that line may break other things. Also, I think probably it should be disabled only on browsers that have compatibility problems, but I am not sure. I do not know why the lang attribute was added in the beginning.

I asked about the browser version to see whether it is possible to have different behaviour based on the browser version. It seems not. The OS version has to be used to determine the behaviour.

goranmoomin commented 4 years ago

@adah1972

Simply removing that line may break other things.

I believe not. Looks like all of the language-related stuff that happens in JavaScript gets it’s locale from navigator.language.

Also, I think probably it should be disabled only on browsers that have compatibility problems, but I am not sure. I do not know why the lang attribute was added in the beginning. ...

I think it’s just for completeness, it’ll be fine to remove it. If it’s not, the maintainers will decide.

Sent with GitHawk