Display distinct error message for missing font. #670
Conversation
Previously when a font specified in user.json's fontFamily property was missing, it was handled as if the font existed but was non-monospace, and a dialog was brought up that displayed information that was not adapted to that situation and potentially confusing. This commit tests for the missing font case separately from the non-monospace font case, and it causes a different and more appropriate dialog to be displayed in that case. This commit also tests whether the user selected font supports kerning or ligatures, and if so displays the same error message displayed for propotional fonts: It is rare and usually a bug when it happens (as in https://github.com/googlei18n/noto-fonts/issues/1021), but it is possible for a monospace font to have ligature or kerning pairs, and if that happens this will cause issues similar to those that come from using a proprotional font. This commit expands the font metric tests to detect and handle that error case.
|
Do kerning and ligatures break Ace rendering? |
|
They don't break rendering but they break cursor positioning. Try settings "Noto Sans Mono" as the font (this may need to be installed if not running on Chrome OS), then enter text with a ligature like "fifi", then try to enter some text between the first and the second "fi" in that text to see the cursor problem. This is probably not a big deal because monospace fonts typically don't have kerning or ligatures defined. For example, it seems to be a bug in the case of Noto Sans Mono (https://github.com/googlei18n/noto-fonts/issues/1021). On the other hand testing for ligatures and kerning only involves a few additional characters in the code (and it does work, I tested with Noto Sans Mono). The biggest impact is the length of the explanation in the comment. It's your judgment call as to whether handling that case is useful... I can take it out if you want. |
|
No, no, it's fine. I've just never tested it. In general, the edge cases from the font-handling are always a mystery to me, because I've never understood what causes people to set weird fonts for Caret anyway. |
The commit in this pull request causes a more appropriate message to be displayed if a user-specified font is missing. Currently the proportional-font error message is display in that case, which can be confusing.
It also adds tests for font kerning and ligatures, which can get in the way of cursor positioning in the same way that proportional fonts can. See the commit message and code comments for more details.
Finally, it makes the font tests use a canvas object instead of adding a element to the DOM. This is slightly simpler and should be a bit faster (though specific performance tests have not been run).
The results of the change have been tested on Chrome OS running the latest stable version of Chrome.