Fixing ShowInTaskbar=false doesn't work in some cases (#6421)#6989
Fixing ShowInTaskbar=false doesn't work in some cases (#6421)#6989Tanya-Solyanik merged 8 commits intomainfrom unknown repository
Conversation
|
Looks good, could you please add unit test for this case |
|
@Tanya-Solyanik Thank you. I have tested it on my machine. I am new to this, do you have a spec document so I can learn how to make unit tests that conform to the .net spec? Because this change actually only adjusts one line of the statement. I'm not sure about the test system's rules for dealing with such changes. |
|
@roland5572 - you could use this https://github.com/dotnet/winforms/blob/main/docs/testing.md as a starting point. |
|
@Tanya-Solyanik Thank you. Do you prefer a unit test for current issue, or a unit test specifically tests a function that has been fixed? |
Ha-ha, may I have both please? I meant the current issue. We are aiming to accompany fixes with "regression" tests that cover specific issues being fixed. But if you have time to cover this functionality, such a PR will always be welcome. |
|
Thank you. I am learning to make unit tests. I'll try to get it done over the weekend. |
|
I'm currently OOF, and unable to review. Just want to note that if tests need to show forms, those tests should be thought of as integration tests. And the best place for those is UIIntegrationTests project. Before adding an automated test please start with a manual test, it should be added to WinFormsControlTests project (writing project names from memory).
One of the best ways to approach a complex issue is to write a test (and have it failed) in the first commit, then add a fix in a second commit (at this point the test must pass). This approach works great if you're new to testing as well.
|
|
|
|
Thank you, I have added unit test. |
src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/FormTests.cs
Outdated
Show resolved
Hide resolved
src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/FormTests.cs
Outdated
Show resolved
Hide resolved
src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/FormTests.cs
Outdated
Show resolved
Hide resolved
src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/FormTests.cs
Outdated
Show resolved
Hide resolved
|
|
||
| control.Load += (object sender, EventArgs e) => | ||
| { | ||
| control.ShowInTaskbar = false; |
There was a problem hiding this comment.
Is it possible to confirm that handle had been re-created? Perhaps compare handle values? Or read the state?
There was a problem hiding this comment.
In this Issue, Handle is bound to be created successfully. This Issue is not caused by the failure to create the Handle, but in the process of creating the Handle, a statement calls the statement that automatically closes the window. If you tend to compare handles, I'll add that.
|
|
||
| control.ShowDialog(); | ||
|
|
||
| Assert.Equal(prefValue, control.DialogResult); |
There was a problem hiding this comment.
Please confirm that the handle had been created.
src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/FormTests.cs
Outdated
Show resolved
Hide resolved
|
@Tanya-Solyanik Thank you. I have modified all the code according to your suggestion, and added a manual test. |
src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/FormTests.cs
Outdated
Show resolved
Hide resolved
Tanya-Solyanik
left a comment
There was a problem hiding this comment.
Thank you, Looks good
|
Will this go into a servicing release for .NET 6 @Tanya-Solyanik? |
|
@AraHaan , given this has been the behavior since .NET framework, it wouldn't meet the servicing bar unless there is a business justification on customer impact. |
|
So basically winforms programs targeting .NET 6 (and might be production quality) will still be broken by this as it won't be serviced as they cannot target .NET 7 yet until RC at least? |
Since this was an existing issue apps targeting .NET 6.0 aren't affected by this issue any more than apps targeting .NET Framework. |
Fixes #6421.
If you set ShowInTaskbar=false, the winforms will recreate the Handle. It will call RecreateHandleCore() from Control.cs.
In method WmNCDestroy(ref Message m) from class Form.cs, The program needs to confirm whether the handle is being recreated before setting the value of the DialogResult.
If the program is in the process of recreating the handle, setting DialogResult = DialogResult.Cancel will cause the Form to be automatically closed.
Microsoft Reviewers: Open in CodeFlow