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

[regression/8.0.0-preview.4.8333] Crash using Pinvoke.SetParent to create Window as Child #17490

Closed
danielancines opened this issue Sep 19, 2023 · 7 comments · Fixed by #17746
Assignees
Labels
area-controls-window Window fixed-in-8.0.0-rc.2.9511 Look for this fix in 8.0.0-rc.2.9511 fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! high It doesn't work at all, crashes or has a big impact. i/regression This issue described a confirmed regression on a currently supported version p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint platform/windows 🪟 t/bug Something isn't working
Milestone

Comments

@danielancines
Copy link

danielancines commented Sep 19, 2023

Description

I have an application using Pinvoke.SetParent from user32.dll to create an MDI window experience, it's working like a charm on .net 7 last version, but, when I try .net 8 RC1, I'm getting an error and application crashes.

Steps to Reproduce

  1. Create a Maui App;
  2. Remove others targets, let only Windows;
  3. Create a CustomWindow inherited from Window;
  4. Create CustomWindowHandler inherited from WindowHandler;
  5. On ConnectHandler inside the handler, get parent window handle and child window handle;
  6. Call SetParent;
  7. Application crashes;

Link to public reproduction project repository

https://github.com/danielancines/Maui-SetParentError-dotnet8Rc1

Version with bug

8.0.0-preview.4.8333

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.0-preview.3.8149

Affected platforms

Windows

Affected platform versions

windows10.0.19041.0

Did you find any workaround?

No.

Relevant log output

No response

@danielancines danielancines added the t/bug Something isn't working label Sep 19, 2023
@danielancines
Copy link
Author

Testing the same code on Maui at Main branch, the error occurs at GetDisplayDensity on WindowExtensions.

platformWindow.AppWindow is null

image

@lfmouradasilva
Copy link

Same issue here with SetParent when I try .Net 8 RC1

@danielancines danielancines changed the title Crash using Pinvoke.SetParent to create Window as Child [8.0.0-rc.1.9171] Crash using Pinvoke.SetParent to create Window as Child Sep 19, 2023
@LAGAMARIB
Copy link

I had the same problem when trying to update to .net 8

@rodirigos
Copy link

Any updates on this? I also have a similar problem and the company wants the .net8 improvements!

@samhouts samhouts added platform/windows 🪟 high It doesn't work at all, crashes or has a big impact. area-controls-window Window potential-regression This issue described a possible regression on a currently supported version., verification pending labels Sep 19, 2023
@samhouts samhouts added this to the .NET 8 GA milestone Sep 19, 2023
@samhouts samhouts changed the title [8.0.0-rc.1.9171] Crash using Pinvoke.SetParent to create Window as Child [regression/8.0.0-rc.1.9171] Crash using Pinvoke.SetParent to create Window as Child Sep 19, 2023
@samhouts samhouts changed the title [regression/8.0.0-rc.1.9171] Crash using Pinvoke.SetParent to create Window as Child [regression/8.0.0-preview.4.8333] Crash using Pinvoke.SetParent to create Window as Child Sep 19, 2023
@samhouts
Copy link
Member

Confirmed that this regressed between 8.0.0-preview.3.8149 and 8.0.0-preview.4.8333. #14517 seems sus.

@samhouts samhouts added i/regression This issue described a confirmed regression on a currently supported version and removed potential-regression This issue described a possible regression on a currently supported version., verification pending labels Sep 19, 2023
@PureWeen PureWeen modified the milestones: .NET 8 GA, .NET 8 SR1 Sep 27, 2023
@PureWeen
Copy link
Member

It looks like when you call

var mainWindowHandle = (Application.Current.MainPage.Window.Handler.PlatformView as MauiWinUIWindow).GetWindowHandle();
var childWindowHandle = this.PlatformView.GetWindowHandle();

PInvoke.SetParent(new Windows.Win32.Foundation.HWND(childWindowHandle), new Windows.Win32.Foundation.HWND(mainWindowHandle));

The AppWindow is null. We switched over to using AppWindow to customize the toolbar so it looks like we'll need to be aware of this. It looks like we just happen to not need AppWindow for any scenarios you've been running up until now.

rmarinho added a commit that referenced this issue Oct 3, 2023
### Description of Change

When opening a new WinUI window. If the user parents that to the current
window `UI.Xaml.Window.AppWindow` will be null so we need to account for
this.

I've also added `Window.AppWindow` to our set of banned APIs since it
doesn't have null safety. Our `GetAppWindow` API does so that should
keep us in a safer place to not regress this issue.

### Issues Fixed

Fixes #17490
@samhouts samhouts added the p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint label Oct 3, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Nov 2, 2023
@PureWeen
Copy link
Member

@danielancines @lfmouradasilva @LAGAMARIB @rodirigos

What does your scenario look like here where you're using SetParent?
It looks like this approach no longer works in 1.4 and we're wanting to understand your use cases a bit more so we can make sure you all have a path forward.

If you have a chance and can describe your scenarios here #20253

That would be very helpful to us.

@samhouts samhouts added fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! fixed-in-8.0.0-rc.2.9511 Look for this fix in 8.0.0-rc.2.9511 labels Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-window Window fixed-in-8.0.0-rc.2.9511 Look for this fix in 8.0.0-rc.2.9511 fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! high It doesn't work at all, crashes or has a big impact. i/regression This issue described a confirmed regression on a currently supported version p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint platform/windows 🪟 t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants