wpf: add width/height checks when resizing the terminal #7983

javierdlg · 2020-10-20T21:20:37Z

We are getting some watson crash reports that the terminal is attempting
to resize to (0, 0). This change makes it so that we prevent such
resizing and if so, throw an exception before we reach native code.

This commit adds resizing checks that prevent resizing the terminal WPF
control to a size of (0, 0)

WSLUser · 2020-10-21T00:33:50Z

Looks like you are using a fork prior to Super Linter getting added. Can you rebase/merge master to your fork?

DHowett · 2020-10-21T01:38:43Z

Because of the conflict, Javier will need to merge master anyway. Be aware though: the linter doesn't lint C# files, and we use stylecop for that.

zadjii-msft · 2020-10-21T07:39:59Z

. Be aware though: the linter doesn't lint C# files, and we use stylecop for that.

Well, that's sure to conflict, won't it.

zadjii-msft

Does resizing to (0,0) cause the crashes, or does resizing to 0 in either dimension (i.e. (w,0) or (0,h)) also cause a crash?

javierdlg · 2020-10-21T21:43:21Z

@zadjii-msft I haven't tested (0, #) but when set to (0,0) it crashes VS since an unhandled exception is raised in the native side. We've added guards for this on the VS side but I thought we'd do it here as well.
We got the details via a Watson dump and you can find more information and the dump here

DHowett · 2020-10-22T00:02:09Z

src/cascadia/WpfTerminalControl/TerminalContainer.cs

+                throw new ArgumentException(nameof(renderSize), "Terminal column or row count cannot be 0.");
+            }
+
+            var dpiScale = VisualTreeHelper.GetDpi(this);


i don't see why we need this line..

Looks like it came from a bad merge. Pushing a fix...

DHowett

Just the one question

ghost · 2020-10-27T01:57:06Z

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

We are getting some watson crash reports that the terminal is attempting to resize to `(0, 0)`. This change makes it so that we prevent such resizing and if so, throw an exception before we reach native code. This commit adds resizing checks that prevent resizing the terminal WPF control to a size of `(0, 0)` (cherry picked from commit 5a518e5)

ghost · 2020-11-11T18:04:15Z

🎉Windows Terminal v1.4.3141.0 has been released which incorporates this pull request.:tada:

Handy links:

ghost · 2020-11-11T18:08:02Z

🎉Windows Terminal Preview v1.5.3142.0 has been released which incorporates this pull request.:tada:

Handy links:

Added checks when resizing the terminal

3088446

javierdlg requested a review from miniksa October 20, 2020 21:23

RyanToth3 approved these changes Oct 20, 2020

View reviewed changes

zadjii-msft approved these changes Oct 21, 2020

View reviewed changes

zadjii-msft added Issue-Bug It either shouldn't be doing this or needs an investigation. Severity-Crash Crashes are real bad news. Area-WPFControl Things related to the WPF version of the TermControl labels Oct 21, 2020

Merge branch 'master' into dev/jadelaga/PreventInvalidResize

9169443

javierdlg requested a review from DHowett October 21, 2020 22:39

DHowett reviewed Oct 22, 2020

View reviewed changes

DHowett requested changes Oct 22, 2020

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 22, 2020

microsoft-github-updates bot changed the base branch from master to main October 22, 2020 00:26

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Oct 23, 2020

Removed bad merge line

5b0ba5c

DHowett approved these changes Oct 27, 2020

View reviewed changes

DHowett changed the title ~~Added checks when resizing the terminal~~ wpf: add width/height checks when resizing the terminal Oct 27, 2020

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Oct 27, 2020

ghost merged commit 5a518e5 into main Oct 27, 2020

ghost deleted the dev/jadelaga/PreventInvalidResize branch October 27, 2020 01:57

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wpf: add width/height checks when resizing the terminal #7983

wpf: add width/height checks when resizing the terminal #7983

javierdlg commented Oct 20, 2020 •

edited by DHowett

WSLUser commented Oct 21, 2020

DHowett commented Oct 21, 2020

zadjii-msft commented Oct 21, 2020

zadjii-msft left a comment

javierdlg commented Oct 21, 2020 •

edited

DHowett Oct 22, 2020

javierdlg Oct 23, 2020

DHowett left a comment

ghost commented Oct 27, 2020

ghost commented Nov 11, 2020

ghost commented Nov 11, 2020

wpf: add width/height checks when resizing the terminal #7983

wpf: add width/height checks when resizing the terminal #7983

Conversation

javierdlg commented Oct 20, 2020 • edited by DHowett

WSLUser commented Oct 21, 2020

DHowett commented Oct 21, 2020

zadjii-msft commented Oct 21, 2020

zadjii-msft left a comment

Choose a reason for hiding this comment

javierdlg commented Oct 21, 2020 • edited

DHowett Oct 22, 2020

Choose a reason for hiding this comment

javierdlg Oct 23, 2020

Choose a reason for hiding this comment

DHowett left a comment

Choose a reason for hiding this comment

ghost commented Oct 27, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

ghost commented Nov 11, 2020

ghost commented Nov 11, 2020

javierdlg commented Oct 20, 2020 •

edited by DHowett

javierdlg commented Oct 21, 2020 •

edited

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.