Specifying a font with name longer than 32 characters causes the app to crash #602
Labels
Area-Fonts
Related to the font
Issue-Bug
It either shouldn't be doing this or needs an investigation.
Needs-Tag-Fix
Doesn't match tag requirements
Product-Terminal
The new Windows Terminal.
Resolution-Fix-Committed
Fix is checked in, but it might be 3-4 weeks until a release.
Severity-Crash
Crashes are real bad news.
Milestone
Comments
|
Alas, it’s true. Thanks for the report!
…________________________________
From: luk1337 <notifications@github.com>
Sent: Wednesday, May 8, 2019 3:17:58 PM
To: microsoft/Terminal
Cc: Subscribed
Subject: [microsoft/Terminal] Specifying a font with name longer than 32 characters causes the app to crash (#602)
The font handling code uses LF_FACESIZE define from wingdi.h which is set to 32, however some fonts like "Droid Sans Mono Dotted for Powerline" use name longer than that which makes the Terminal app crash when trying to load font.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2FTerminal%2Fissues%2F602&data=01%7C01%7Cduhowett%40microsoft.com%7C33a2886931ab4ba559fc08d6d4030741%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=8p8HtsfYgH55%2FikZhch1BqknprXpYbfLa1sjzV6V%2Fto%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FADNHLGVXI5MHSPIQGIKP2KDPUNGRNANCNFSM4HLVUXAA&data=01%7C01%7Cduhowett%40microsoft.com%7C33a2886931ab4ba559fc08d6d4030741%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=ROr4XMTNuP2CxAfZQzpIE%2BAv5MW0RitYRjgn8KXlZjc%3D&reserved=0>.
|
DHowett-MSFT
added
Area-Fonts
|
This will require us to tease apart the |
This was referenced Sep 5, 2019
DHowett-MSFT
pushed a commit
that referenced
this issue
Oct 7, 2019
We now truncate the font name as it goes out to GDI APIs, in console API servicing, and in the propsheet. Fixes #602.
5 tasks
DHowett-MSFT
pushed a commit
that referenced
this issue
Oct 15, 2019
We now truncate the font name as it goes out to GDI APIs, in console API servicing, and in the propsheet. I attempted to defer truncating the font to as far up the stack as possible, so as to make FontInfo usable for the broadest set of cases. There were a couple questions that came up: I know that `Settings` gets memset (memsat?) by the registry deserializer, and perhaps that's another place for us to tackle. Right now, this pull request enables fonts whose names are >= 32 characters _in Windows Terminal only_, but the underpinnings are there for conhost as well. We'd need to explicitly break at the API, or perhaps return a failure or log something to telemetry. * Should we log truncation at the API boundary to telemetry? -> Later; followup filed (#3123) * Should we fix Settings here, or later? -> Later; followup filed (#3123) * `TrueTypeFontList` is built out of things in winconp, the private console header. Concern about interop structures. -> Not used for interop, followup filed to clean it up (#3123) * Is `unsigned int` right for codepage? For width? -> Yes: codepage became UINT (from WORD) when we moved from Win16 to Win32 This commit also includes a workaround for #3170. Growing CONSOLE_INFORMATION made us lose the struct layout lottery during release builds, and this was an expedient fix. Closes #602. Related to #3123.
ghost
added
Needs-Tag-Fix
|
🎉This issue was addressed in #3107, which has now been successfully released as Handy links: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The font handling code uses LF_FACESIZE define from wingdi.h which is set to 32, however some fonts like "Droid Sans Mono Dotted for Powerline" use name longer than that which makes the Terminal app crash when trying to load font.
The text was updated successfully, but these errors were encountered: