External Dialogs not visible with MainWindow set to IgnoreTaskbarOnMaximize #2780

Closed
fjlaga opened this Issue Dec 24, 2016 · 9 comments

Comments

Projects
None yet
2 participants
@fjlaga

fjlaga commented Dec 24, 2016

What steps will reproduce this issue?

This issue is happening in my application after I upgraded to 1.4.0. It is reproducible in the demo app by following these steps:

  1. Run the MahApp.Metro Demo .Net45
  2. Select the Window > Ignore Taskbar on Maximize option
  3. Maximize the main window
  4. Select Dialogs > Show ShowInputDialog Externally

The main window will dim and disable, the ShowInputDialog will appear briefly, then disappear behind the main window (which remains dim and disabled).

Expected outcome

The ShowInputDialog does not disappear behind the dim and disabled main window.

--

Environment

  • MahApps.Metro v 1.4.0
  • Windows 10
  • Visual Studio 2015
  • .NET Framework 4.6
@fjlaga

This comment has been minimized.

Show comment
Hide comment
@fjlaga

fjlaga Dec 27, 2016

I've found the cause and a fix for this issue...

The fix for #2694 set the Z-Order of a window to HWND_BOTTOM, HWND_TOP, then blindly set it to HWND_NOTOPMOST. If the window is an external dialog, this is no good. The Z-Order of an external dialog must be set to be after the Owner window.

I've fixed this by adding the following at line 377 in BorderlessWindowBehavior.cs:

// #2780 Don't blindly set the Z-Order to HWWND_NOTOPMOST. If this window has an owner, set
// the Z-Order to be after the owner. This keeps external dialogs appearing correctly above
// their owner window even when owner window is maximized and ignoring taskbar.
IntPtr hwndInsAfter = Constants.HWND_NOTOPMOST;
if (metroWindow.Owner != null)
hwndInsAfter = new WindowInteropHelper(metroWindow.Owner).Handle;
UnsafeNativeMethods.SetWindowPos(this.handle, hwndInsAfter, left, top, width, height, Constants.TOPMOST_FLAGS);

I'm still trying to figure out how to make a pull request. It may be faster for someone else to make the change. Let me know if I should make a pull request, but I may need help with that.

fjlaga commented Dec 27, 2016

I've found the cause and a fix for this issue...

The fix for #2694 set the Z-Order of a window to HWND_BOTTOM, HWND_TOP, then blindly set it to HWND_NOTOPMOST. If the window is an external dialog, this is no good. The Z-Order of an external dialog must be set to be after the Owner window.

I've fixed this by adding the following at line 377 in BorderlessWindowBehavior.cs:

// #2780 Don't blindly set the Z-Order to HWWND_NOTOPMOST. If this window has an owner, set
// the Z-Order to be after the owner. This keeps external dialogs appearing correctly above
// their owner window even when owner window is maximized and ignoring taskbar.
IntPtr hwndInsAfter = Constants.HWND_NOTOPMOST;
if (metroWindow.Owner != null)
hwndInsAfter = new WindowInteropHelper(metroWindow.Owner).Handle;
UnsafeNativeMethods.SetWindowPos(this.handle, hwndInsAfter, left, top, width, height, Constants.TOPMOST_FLAGS);

I'm still trying to figure out how to make a pull request. It may be faster for someone else to make the change. Let me know if I should make a pull request, but I may need help with that.

@punker76 punker76 added this to the 1.4.1 milestone Jan 5, 2017

@punker76 punker76 added the Bug label Jan 5, 2017

@punker76 punker76 closed this in 513b76c Jan 5, 2017

@fjlaga

This comment has been minimized.

Show comment
Hide comment
@fjlaga

fjlaga Jan 9, 2017

@punker76 Thanks for applying the fix based on my comments...unfortunately, the fix was not quite right and there is some other issues now when external dialogs are shown. I should have been more clear in my message. Better yet, I should have just made a Pull Request.

I have cloned the repo, created a branch, committed my changes to fix this...but I can't seem to push my changes and create a pull request. I'm new to Git so I will need some help here. Can you point me to some wiki or something I can read to figure out how to create a pull request for my changes?

fjlaga commented Jan 9, 2017

@punker76 Thanks for applying the fix based on my comments...unfortunately, the fix was not quite right and there is some other issues now when external dialogs are shown. I should have been more clear in my message. Better yet, I should have just made a Pull Request.

I have cloned the repo, created a branch, committed my changes to fix this...but I can't seem to push my changes and create a pull request. I'm new to Git so I will need some help here. Can you point me to some wiki or something I can read to figure out how to create a pull request for my changes?

@punker76

This comment has been minimized.

Show comment
Hide comment
@punker76

punker76 Jan 9, 2017

Member

@fjlaga I don't see other issues, tested all external dialogs...

Member

punker76 commented Jan 9, 2017

@fjlaga I don't see other issues, tested all external dialogs...

@fjlaga

This comment has been minimized.

Show comment
Hide comment
@fjlaga

fjlaga Jan 9, 2017

@punker76 There are no other issues in the demo app. I am seeing issues in my app where we use external dialogs much more. There are two issues with the fix you submitted:

  1. You removed two calls to SetWindowPos which were for fix #2694. These calls set the z-order to HWND_BOTTOM, then to HWND_TOP, before setting it to HWND_NOTOPMOST. My fix was to replace only the last SetWindowPos call to use the owner window instead of HWND_NOTOPMOST. I'm not sure removing the two calls to SetWindowPos actually causes a problem, but the author of the fix for #2694 thought those two calls were important, so I didn't want to take them out.

  2. My fix for #2780 changed the last call from using HWND_NOTOPMOST to the owner window, but it used the SWP flags Constants.TOPMOST_FLAGS. Your fix changes this to use 0x0040 which is the same as SWP_SHOWWINDOW. This is causing problems in my app...I have some custom dialogs that are used more or less like tooltip windows. With your fix, the main window flashes active/inactive every time one of these custom tooltip windows is shown.

I will work on a pull request.

fjlaga commented Jan 9, 2017

@punker76 There are no other issues in the demo app. I am seeing issues in my app where we use external dialogs much more. There are two issues with the fix you submitted:

  1. You removed two calls to SetWindowPos which were for fix #2694. These calls set the z-order to HWND_BOTTOM, then to HWND_TOP, before setting it to HWND_NOTOPMOST. My fix was to replace only the last SetWindowPos call to use the owner window instead of HWND_NOTOPMOST. I'm not sure removing the two calls to SetWindowPos actually causes a problem, but the author of the fix for #2694 thought those two calls were important, so I didn't want to take them out.

  2. My fix for #2780 changed the last call from using HWND_NOTOPMOST to the owner window, but it used the SWP flags Constants.TOPMOST_FLAGS. Your fix changes this to use 0x0040 which is the same as SWP_SHOWWINDOW. This is causing problems in my app...I have some custom dialogs that are used more or less like tooltip windows. With your fix, the main window flashes active/inactive every time one of these custom tooltip windows is shown.

I will work on a pull request.

@punker76

This comment has been minimized.

Show comment
Hide comment
@punker76

punker76 Jan 9, 2017

Member

@fjlaga It would be very helpful if you can create an app with such a 'tooltip' to reproduce this. I don't know if it's a good idea to use dialogs as tooltips, but a sample could be help to look what's going on...

  1. to this point, I was the author of the fix...
  2. create a sample to show your custom dialog tooltip, so we can look to this problems

thx

Member

punker76 commented Jan 9, 2017

@fjlaga It would be very helpful if you can create an app with such a 'tooltip' to reproduce this. I don't know if it's a good idea to use dialogs as tooltips, but a sample could be help to look what's going on...

  1. to this point, I was the author of the fix...
  2. create a sample to show your custom dialog tooltip, so we can look to this problems

thx

@fjlaga

This comment has been minimized.

Show comment
Hide comment
@fjlaga

fjlaga Jan 11, 2017

@punker76 I haven't had time to create a sample app that demonstrates the error. I have cloned the repo and made the changes that will fix it. The changes are minor and I think they can simply be reviewed without a sample app. My problem is when I try to push my changes I am denied access.

Error encountered while pushing to the remote repository: Response status code does not indicate success: 403 (Forbidden).

Since I can't push my changes, I can't create a pull request.

A little help would be appreciated.

fjlaga commented Jan 11, 2017

@punker76 I haven't had time to create a sample app that demonstrates the error. I have cloned the repo and made the changes that will fix it. The changes are minor and I think they can simply be reviewed without a sample app. My problem is when I try to push my changes I am denied access.

Error encountered while pushing to the remote repository: Response status code does not indicate success: 403 (Forbidden).

Since I can't push my changes, I can't create a pull request.

A little help would be appreciated.

@punker76

This comment has been minimized.

Show comment
Hide comment
@punker76

punker76 Jan 11, 2017

Member

I haven't had time to create a sample app that demonstrates the error. I have cloned the repo and made >the changes that will fix it. The changes are minor and I think they can simply be reviewed without a >sample app.

That's not the point. Yes I can review it, but you say that you get issues at your app and not at the main MahApps demo. So how can I find out what's going on? Maybe it's easy enough, maybe it has some other side effects...

My problem is when I try to push my changes I am denied access.

Error encountered while pushing to the remote repository: Response status code does not indicate >success: 403 (Forbidden).

It seems you didn't forked the repo.

To create a PR you must do the following steps

  • fork the repo
  • clone your fork to your local disk
  • create a branch Fix-for-XYZ
  • make your changes
  • commit your changes in your branch
  • push you branch on your repo
  • now you can make a PR from your branch to the dev branch of MahApps

hope this helps

Member

punker76 commented Jan 11, 2017

I haven't had time to create a sample app that demonstrates the error. I have cloned the repo and made >the changes that will fix it. The changes are minor and I think they can simply be reviewed without a >sample app.

That's not the point. Yes I can review it, but you say that you get issues at your app and not at the main MahApps demo. So how can I find out what's going on? Maybe it's easy enough, maybe it has some other side effects...

My problem is when I try to push my changes I am denied access.

Error encountered while pushing to the remote repository: Response status code does not indicate >success: 403 (Forbidden).

It seems you didn't forked the repo.

To create a PR you must do the following steps

  • fork the repo
  • clone your fork to your local disk
  • create a branch Fix-for-XYZ
  • make your changes
  • commit your changes in your branch
  • push you branch on your repo
  • now you can make a PR from your branch to the dev branch of MahApps

hope this helps

fjlaga added a commit to fjlaga/MahApps.Metro that referenced this issue Jan 12, 2017

@fjlaga

This comment has been minimized.

Show comment
Hide comment
@fjlaga

fjlaga Jan 12, 2017

fjlaga commented Jan 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment