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.

Sign up for GitHub

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

Jump to bottom

Display a dialog when the user selects an invalid font #1017

Closed
matt2405warner opened this issue May 25, 2019 · 11 comments · Fixed by #8207
Closed

Display a dialog when the user selects an invalid font #1017

matt2405warner opened this issue May 25, 2019 · 11 comments · Fixed by #8207
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v2.0

Comments

@matt2405warner
Copy link

Summary of the new feature/enhancement

Currently when changing the 'fontFace' to a font that doesn't exist the font size just gets so small (~0.5 font size) that you can't read the text.

Proposed technical implementation details (optional)

It would be nice if the terminal informed the user that the font is not valid - Maybe a pop-up/modal?
@matt2405warner matt2405warner added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label May 25, 2019
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 25, 2019
@metathinker
Copy link
Contributor

I also see an earlier report (#550) that Terminal would actually crash when a profile referred to a nonexistent font name. Is that not happening anymore? Perhaps that problem was partially fixed, resulting in what you see.

@matt2405warner
Copy link
Author

I haven't had it crash on me yet when I change the font to an invalid one. It just makes the font extremely small.

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels May 28, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 28, 2019
@zadjii-msft
Copy link
Member

Well this is definitely relevant to #610, @miniksa might be interested in knowing that. I was certainly under the impression we'd crash when the font wasn't found.

@miniksa
Copy link
Member

miniksa commented May 28, 2019

I mean, literally anyone who isn't me can go F5 the thing in Visual Studio setting a breakpoint in dxrenderer.cpp where it hands your font preference into DirectWrite's layout and see what it comes back with to reason out why this is. I think it's GetProposedFont method.

But if no one else does that, I'll get to this eventually.

@DHowett-MSFT DHowett-MSFT added Help Wanted We encourage anyone to jump in on these. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 28, 2019
@metathinker
Copy link
Contributor

Well, for what it's worth, I can't reproduce this problem. I gave a profile a nonexistent font name and Terminal crashed as per #550 when creating a tab with that profile; it didn't show a tab with tiny text as you report here.

@matt2405warner
Copy link
Author

@metathinker this occurred when I had a terminal tab already open when I was editing the profile. @zadjii-msft are you at least informing the user why the terminal crashed?

@zadjii-msft
Copy link
Member

@matt2405warner considering the terminal was hard crashing, I don't think the app had a chance to catch the exception that triggered the crash. Now that #903 is complete, we certainly can add another message here

@zadjii-msft zadjii-msft added this to the Terminal v1.0 milestone Jun 19, 2019
@DHowett-MSFT DHowett-MSFT mentioned this issue Jun 24, 2019
Closed
@zadjii-msft zadjii-msft changed the title Feature Request: Invalid Font Display a dialog when the user selects an invalid font Jul 11, 2019
@zadjii-msft zadjii-msft mentioned this issue Jul 11, 2019
Closed
@kmoberg
Copy link

kmoberg commented Jul 31, 2019

I also see an earlier report (#550) that Terminal would actually crash when a profile referred to a nonexistent font name. Is that not happening anymore? Perhaps that problem was partially fixed, resulting in what you see.

I've tried changing to a non-existent font today, and it hard-crashes the entire application when I do.

@miniksa miniksa removed the Help Wanted We encourage anyone to jump in on these. label Jul 31, 2019
@miniksa
Copy link
Member

miniksa commented Jul 31, 2019

@kmoberg, I fixed #550 19 hours ago. It'll be in the next release or you can build from master to get the fix.

However, the way I fixed it just resulted in the DX renderer trying things until it can find a font. If we're to add a dialog too, we probably need to detect that the requested font doesn't match the returned font and display.

I can take the issue from here.

@miniksa miniksa self-assigned this Jul 31, 2019
@offero
Copy link

offero commented Aug 6, 2019

How do you find the string that represents the font variant you want?

@zadjii-msft zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) and removed Issue-Task It's a feature request, but it doesn't really need a major design. labels Nov 18, 2019
@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Aug 13, 2020
@zadjii-msft zadjii-msft mentioned this issue Aug 13, 2020
Closed
This was referenced Nov 6, 2020
Closed
Merged
@ghost ghost added the In-PR This issue has a related PR label Nov 9, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Nov 11, 2020
DHowett pushed a commit that referenced this issue Nov 11, 2020
@miniksa
Warn when font isn't found and another is chosen (#8207)
4998779 
Display a warning message when the DirectX renderer resolves a font that
isn't the one you selected to warn that it couldn't be found.

Also I wrote the dialog event chain out of `TermControl` to be reusable
in the future for other messages the control might want to tell a host
about and various levels. 

## Validation Steps Performed
- Manual validation, setting bad font name, fixing font name with
  `settings.json`.

Closes #1017
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Nov 11, 2020
@ghost
Copy link

ghost commented Jan 28, 2021

🎉This issue was addressed in #8207, which has now been successfully released as Windows Terminal Preview v1.6.10272.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-3 A description (P3) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v2.0
Development

Successfully merging a pull request may close this issue.

Warn when font isn't found and another is chosen microsoft/terminal
8 participants
@kmoberg @metathinker @offero @DHowett-MSFT @matt2405warner @miniksa @zadjii-msft @cinnamon-msft