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.

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

Form.ActiveForm does not return null when an owned model window should not be "active" #3003

Open
AraHaan opened this issue Mar 21, 2020 · 19 comments
Milestone

Comments

@AraHaan
Copy link
Contributor

@AraHaan AraHaan commented Mar 21, 2020

  • .NET Core Version:

.NET Core 3.1

  • Have you experienced this same bug with .NET Framework?:

Yes

Problem description:

When a form is showed the ActiveForm property on a form returns the form (if main form returns that), or null when a window from another process is active. However when on a Model form with the main form as the owner, doing that results in the property always returning that Model form instead of null like I think it should. Not even the Deactivate nor the Activated events fire when this happens.

Expected behavior:
for ActiveForm to return null when no forms (including model forms with the main form as owner) when a window from another application is active. Same for the Activated/Deactivate events should also fire as well when this happens or when reclicked on that should make it become Activated again.

Minimal repro:

  1. Create a new winforms project.
  2. Create a in the Form1 code make the form create another form a button in it is clicked making that form the owner of another instance of itself.
  3. Do some painting work that uses the Paint event on the form code that changes colors when
// this.active is an added attempt to double check and make sure it is
// active using the Activated/Deactivate events.
if (Equals(ActiveForm, this) && this.active)
{
    // e.Graphics stuff.
}
else
{
    // e.Graphics stuff.
}
@AraHaan

This comment has been minimized.

Copy link
Contributor Author

@AraHaan AraHaan commented Mar 21, 2020

cc: @RussKie

@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 21, 2020

Have you experienced this same bug with .NET Framework?: Yes

I just tested this and ActiveForm goes to null on Desktop Framework when I put focus to another processes window like you describe in your first paragraph, so obviously I'm not replicating your scenario correctly. Could you provide an example snippet of what you are doing? In particular I have a hard time parsing

Create a in the Form1 code make the form create another form a button in it is clicked making that form the owner of another instance of itself.

Also "model form" is not a technical term in the WinForms framework, maybe you mean "modal form"?

@AraHaan

This comment has been minimized.

Copy link
Contributor Author

@AraHaan AraHaan commented Mar 21, 2020

I mean model form as in a form as an owner that when you try to click on the owner of it it will flicker with a sound on windows and diassallow focus on the owner form until that model form is closed.
and I meant on the Form1 code make the form create another instance of it when the button on it is clicked. I remember that when dotnet new winforms is called it creates Form1 At first I was trying to type to create the 1st form before I remembered that it automatically does it for you.

@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 21, 2020

Yeah ok so you mean showing a modal form. Unfortunately I cannot replicate what you describe, please share a snippet.

With this example code the ActiveForm goes to null when focus leaves the modal form. (Note that I'm trying the repro in Desktop Framework first because that is my reference, since you mentioned you can replicate this in Desktop Framework the first step is to figure out whether what you see is an actual bug or not.)

example
    public partial class Form3 : Form
    {
        public Form3()
        {
            InitializeComponent();

            Name = "Form #" + Application.OpenForms.Count;
        }

        private void button1_Click(object sender, EventArgs e)
        {
            using (var f3 = new Form3())
                f3.ShowDialog(this);
        }

        private void Form3_Activated(object sender, EventArgs e)
        {
            Debug.WriteLine("Form3_Activated");
        }

        private void Form3_Deactivate(object sender, EventArgs e)
        {
            Debug.WriteLine("Form3_Deactivate");
        }

        private void timer1_Tick(object sender, EventArgs e)
        {
            Debug.WriteLine("Active Form: " + (Form.ActiveForm?.Name ?? "<null>"));
        }
    }
@AraHaan

This comment has been minimized.

Copy link
Contributor Author

@AraHaan AraHaan commented Mar 21, 2020

Alright so since I got it on github as well the problem github is: https://github.com/Elskom/Els_kom_new
(just pushed the commit with the issue in particular) The issue is in Controls/ThemedForm which the forms in Forms subclass from that.

But it depends on packages from: https://github.com/AraHaan/MyPackages that needs to be cloned recursively and built with dotnet build --configuration Release --ignore-failed-sources

@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 21, 2020

Hmm I tried to take a look but you depend on preview packages not present in the normal nuget feeds and I don't know what to add to get things like Microsoft.Build.Framework version 16.6.0-preview-20162-03 to resolve. If its nontrivial to build your project on a machine not set up for it you might want to try reducing the problem into a standalone project by removing or stubbing out unnecessary operations while still keeping the problematic behavior intact.

@RussKie

This comment has been minimized.

Copy link
Member

@RussKie RussKie commented Mar 22, 2020

@AraHaan please provide a small repro that does not involve 3rd party packages.

@AraHaan

This comment has been minimized.

Copy link
Contributor Author

@AraHaan AraHaan commented Mar 22, 2020

TestFormActiveForm.zip
Alright here is a zip without any nuget packages. It seems for me that now on this project when trying to reproduce it that now it does that bug on the initial instance as well for some reason too.

weltkante added a commit to weltkante/TestFormActiveForm that referenced this issue Mar 22, 2020
@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 22, 2020

Thanks, for easier discussion than sending zips back and forth I put it in a repo (going to delete it after this issue is resolved)

I added some diagnostics similar to what I posted in my snippet above and everything seems to behave as expected. When I click outside the form (e.g. on the taskbar) I get a deactivation event and ActiveForm goes to null. This happens regardless of whether the initial form is open or I clicked on the button and a modal form shows.

Maybe you can point me at what you think is behaving incorrectly? Feel free to make PRs against it or commit into a fork if it requires code changes to highlight the problem.

@AraHaan

This comment has been minimized.

Copy link
Contributor Author

@AraHaan AraHaan commented Mar 23, 2020

Interesting it seems that activeform code works perfectly in the timer, it might just be my if checking if it is an active window or not is messing up in the Paint event code.

@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 23, 2020

Added code to log paint events. Looks like (de)activation doesn't trigger a paint, which makes sense, because it only affects the frame and painting that goes by a different window message in the native win32 implementation.

Adding explicit invalidation seems to make your forms change color appropriately?

@AraHaan

This comment has been minimized.

Copy link
Contributor Author

@AraHaan AraHaan commented Mar 23, 2020

Also I noticed as well that it seems while the result code is 0 from NativeMethods.SendMessage that the sent message does nothing on the form (on the close and help buttons). I changed the MouseDown event to:

        private void ThemedForm_MouseDown(object sender, MouseEventArgs e)
        {
            if (this.ControlBox)
            {
                // todo: do what the buttons originally did.
                if (this.MinimizeHitbox.Contains(e.Location) && this.MinimizeBox)
                {
                    this.WindowState = FormWindowState.Minimized;
                }
                else if (this.MaximizeHitbox.Contains(e.Location) && this.MaximizeBox)
                {
                    this.WindowState = FormWindowState.Maximized;
                }
                else if (this.HelpHitbox.Contains(e.Location) && this.HelpButton)
                {
                    var result = NativeMethods.SendMessage(this.Handle, (uint)WindowsMessages.SYSCOMMAND, (int)SYSCOMMANDS.SC_CONTEXTHELP, 0);
                    Debug.WriteLineIf(result != 0, $"NativeMethods.SendMessage() failed with error code {result}");
                }
                else if (this.CloseHitbox.Contains(e.Location))
                {
                    var result = NativeMethods.SendMessage(this.Handle, (uint)WindowsMessages.SYSCOMMAND, (int)SYSCOMMANDS.SC_CLOSE, 0);
                    Debug.WriteLineIf(result != 0, $"NativeMethods.SendMessage() failed with error code {result}");
                }
            }
        }

If you debug the close button and placed a breakpoint on the WriteLineIf you will see it returns no issues but does nothing to close the form (with the events for it being broadcasted). Same way for the help button to broadcast those events and to show that ? at the bottom of the cursor.

I think this is in part because of FormWindowState does not have a state for help button pressed sadly too (same for close button for the people like me who need to override the original window frame).

@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 23, 2020

You're right, WM_SYSCOMMAND+SC_CLOSE doesn't work in WinForms, kinda weird since it seems to work in a C/C++ application. Not sure what prevents it from working. I'll see if I can do a quick debug session later today and see if .NET suppresses anything. Its unrelated to your code since I can repro in a fresh project.

PS: your SendMessage were defined wrong, WPARAM and LPARAM are IntPtr, not int - will cause bugs if your process is 64 bit. Its not the cause of the problem though.

@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 23, 2020

You cannot do WM_SYSCOMMAND from WinForms mouse events, might have been possible in the past, but current source uses mouse capture both in Desktop Framework and in .NET Core to track the mouse - and while mouse capture is on all WM_SYSCOMMAND are ignored (I don't think thats documented, but I found people saying that while searching for the problem).

Easiest workaround is to turn capture off, before you call WM_SYSCOMMAND+SC_CLOSE:

this.Capture = false;
SendMessage(this.Handle, WM_SYSCOMMAND, (IntPtr)SC_CLOSE, (IntPtr)0);

For SC_CONTEXTHELP and other commands it might be more complicated because the window isn't going to be closed immediately and you might confuse WinForms if you mess with the capture state and then continue operation as if nothing happened. You'll have to review (or trial-and-error) that yourself, maybe WinForms is robust enough to not care that capture got lost in the middle of a click.

@AraHaan

This comment has been minimized.

Copy link
Contributor Author

@AraHaan AraHaan commented Mar 24, 2020

@weltkante it seems to not care if I switch it to false and then right after sending the message and debugging it on the WriteLineIf to switch it back to true.

Also why (IntPtr)0 and not IntPtr.Zero?

@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 24, 2020

Also why (IntPtr)0 and not IntPtr.Zero?

no particular reason, just what I happened to type

@merriemcgaw merriemcgaw added this to the Future milestone Mar 26, 2020
@RussKie

This comment has been minimized.

Copy link
Member

@RussKie RussKie commented Mar 26, 2020

@AraHaan has @weltkante answered your questions?

@AraHaan

This comment has been minimized.

Copy link
Contributor Author

@AraHaan AraHaan commented Mar 27, 2020

yes he has. However I have a other slight issue sadly with this though. It seems when one double clicks the said hitbox for the icon (that for me case now simulates the WM_SYSCOMMAND menu thing by basically querying it for the handle, manipulating it, and then finally getting the information needed to clone it to a ContestMenuStrip that supports custom renderers) does not send an SC_CLOSE (unlike what windows does when you do that).

@weltkante

This comment has been minimized.

Copy link
Contributor

@weltkante weltkante commented Mar 27, 2020

As far as I understand the documentation, Windows does not send an SC_CLOSE - the menu item is the SC_CLOSE. Getting a WM_SYSCOMMAND is just Windows telling you someone clicked a menu item in the system menu, SC_CLOSE is just the ID of the menu item. The default handling of clicking the menu item will be to close the window. (In other words, WM_SYSCOMMAND+SC_CLOSE is not the reaction to clicking a menu item, it is the event of clicking the system menu item 'SC_CLOSE'. The reaction to clicking that menu item is sending a WM_CLOSE to close the window. You might be mixing up cause and reaction.)

So if you create a custom menu then obviously it no longer is identical to the SC_CLOSE menu item and there won't be any default behavior. Instead of sending an SC_CLOSE windows will call the event that your menu item was clicked.

If you meant to say you were manually sending WM_SYSCOMMAND+SC_CLOSE from a ContextMenuStrip and it didn't work, then it likely is the same reason it didn't work for the form click, someone probably has mouse capture on. The WinForms API only allows you to reset mouse capture if you access it through the property of the control which currently holds capture. If the capture is not held by WinForms or you can't access the control which holds it you could try using native API to force release of the capture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.