[PATCH] quartz: Clear the WS_CHILD style in WM_CLOSE instead of unparenting it before.
We handle it in WM_CLOSE to match Windows more closely, in case the window belongs to a different thread. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48732 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/quartz/window.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dlls/quartz/window.c b/dlls/quartz/window.c index 0b0df13..48466fd 100644 --- a/dlls/quartz/window.c +++ b/dlls/quartz/window.c @@ -79,6 +79,14 @@ static LRESULT CALLBACK WndProcW(HWND hwnd, UINT message, WPARAM wparam, LPARAM This->Width = LOWORD(lparam); This->Height = HIWORD(lparam); + break; + case WM_CLOSE: + /* Media Player Classic deadlocks if WM_PARENTNOTIFY is sent, so clear + * the child style first. Just like Windows, we don't actually unparent + * the window, to prevent extra focus events from being generated since + * it would become top-level for a brief period before being destroyed. */ + SetWindowLongW(hwnd, GWL_STYLE, GetWindowLongW(hwnd, GWL_STYLE) & ~WS_CHILD); + break; } return DefWindowProcW(hwnd, message, wparam, lparam); @@ -136,15 +144,9 @@ HRESULT WINAPI BaseWindowImpl_PrepareWindow(BaseWindow *This) HRESULT WINAPI BaseWindowImpl_DoneWithWindow(BaseWindow *This) { - BaseControlWindow *window = impl_from_BaseWindow(This); - if (!This->hWnd) return S_OK; - /* Media Player Classic deadlocks if WM_PARENTNOTIFY is sent, so unparent - * the window first. */ - IVideoWindow_put_Owner(&window->IVideoWindow_iface, 0); - SendMessageW(This->hWnd, WM_CLOSE, 0, 0); This->hWnd = NULL; -- 2.21.0
On 3/16/20 7:52 AM, Gabriel Ivăncescu wrote:
We handle it in WM_CLOSE to match Windows more closely, in case the window belongs to a different thread.
What's the point of doing this?
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48732 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/quartz/window.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/dlls/quartz/window.c b/dlls/quartz/window.c index 0b0df13..48466fd 100644 --- a/dlls/quartz/window.c +++ b/dlls/quartz/window.c @@ -79,6 +79,14 @@ static LRESULT CALLBACK WndProcW(HWND hwnd, UINT message, WPARAM wparam, LPARAM
This->Width = LOWORD(lparam); This->Height = HIWORD(lparam); + break; + case WM_CLOSE: + /* Media Player Classic deadlocks if WM_PARENTNOTIFY is sent, so clear + * the child style first. Just like Windows, we don't actually unparent + * the window, to prevent extra focus events from being generated since + * it would become top-level for a brief period before being destroyed. */ + SetWindowLongW(hwnd, GWL_STYLE, GetWindowLongW(hwnd, GWL_STYLE) & ~WS_CHILD); + break; }
return DefWindowProcW(hwnd, message, wparam, lparam); @@ -136,15 +144,9 @@ HRESULT WINAPI BaseWindowImpl_PrepareWindow(BaseWindow *This)
HRESULT WINAPI BaseWindowImpl_DoneWithWindow(BaseWindow *This) { - BaseControlWindow *window = impl_from_BaseWindow(This); - if (!This->hWnd) return S_OK;
- /* Media Player Classic deadlocks if WM_PARENTNOTIFY is sent, so unparent - * the window first. */ - IVideoWindow_put_Owner(&window->IVideoWindow_iface, 0); - SendMessageW(This->hWnd, WM_CLOSE, 0, 0); This->hWnd = NULL;
On 16/03/2020 18:00, Zebediah Figura wrote:
On 3/16/20 7:52 AM, Gabriel Ivăncescu wrote:
We handle it in WM_CLOSE to match Windows more closely, in case the window belongs to a different thread.
What's the point of doing this?
Well, based on message sequence I've been investigating, I'm somewhat sure Windows does it when handling a custom message. I know that's internal implementation details, but I figured if we make it closer to Windows by just clearing the style anyway, at least it would be worth it to avoid surprises in the future by also mimicking that behavior. I can drop it, of course, and just handle it like where it was originally. Should I?
On 3/16/20 11:12 AM, Gabriel Ivăncescu wrote:
On 16/03/2020 18:00, Zebediah Figura wrote:
On 3/16/20 7:52 AM, Gabriel Ivăncescu wrote:
We handle it in WM_CLOSE to match Windows more closely, in case the window belongs to a different thread.
What's the point of doing this?
Well, based on message sequence I've been investigating, I'm somewhat sure Windows does it when handling a custom message. I know that's internal implementation details, but I figured if we make it closer to Windows by just clearing the style anyway, at least it would be worth it to avoid surprises in the future by also mimicking that behavior.
I can drop it, of course, and just handle it like where it was originally. Should I?
I don't see a reason to emulate Windows just for the sake of emulating Windows, in a situation like this where the placement doesn't particularly matter. So yeah, I'd prefer this code left where it is, until we get a good reason to change it. While you're at it, you could add a test to show that the parent shouldn't lose focus, by calling e.g. SetActiveWindow() in test_window_threading(), before releasing the filter.
participants (2)
-
Gabriel Ivăncescu -
Zebediah Figura