user32: Fix FlashWindowEx message and return value New

Revisions: 

Revision 3

user image James Coonradt Author
19 Sep. 17

FlashWindowEx is fairly complicated with fake window active states and various timers, but wine currently doesn't simulate any of this.
This patchset makes the message value and return values more sensible and also closer to MSDN documentation:

The message value and return value now reflects the window's active state and not the inverted state or inactive state

This fixes games like Overwatch getting confused about the active window state after calling FlashWindowEx

V2: Add in extra tests to demonstrate that the inverted value is incorrect behavior

V3: Work around a different wine bug by performing some FlashWindowEx tests on the main window, all new tests pass now without todo_wine's and do not introduce new failures.

user image Sebastian Lackner
20 Sep. 17

I hope you are not too disappointed, but I decided to push the release without a fix for this bug. I was hoping to get this
resolved quickly, but the problem turns out to be far more complicated than it looks on the first sight. Adding it in the
next release gives us two more weeks of testing.

all new tests pass now without todo_wine's and do not introduce new failures.

The newly proposed patch is indeed the best one so far, but the tests also do not really cover all situations - for example,
what happens when the next FlashWindowEx call happens while the flashing is still active. In some cases
uCount == 0 also requires special handling and behaves different when a proper count is passed.
I wrote some tests myself (available here: http://ix.io/A2n) and even the latest patch still introduces test failures. In some
cases it might also be caused by Wine bugs, but we have still to be careful that we do not add any new regressions while
trying to fix this. There probably was a reason (for example a specific app) why the previous implementation was written
in such a wrong way.

user image James Coonradt Author
20 Sep. 17

That's fair, the function is fairly complicated in behavior and wine only has a simple implementation. I myself have mostly come to the conclusion that it's not entirely possible to fully implement the original behavior in wine, since the underlying system (X11) doesn't have all the same features and only a basic mark window as needs attention flag. This patch was not intended to fully fix everything but only to be a step in the right direction.

Single files Merged diff Tar archive
You have unsaved changes. Press CTRL + ENTER in a text field to submit your comments.

0001-user32-Fix-FlashWindowEx-message-and-return-value.patch (1 comment)

From 6a1b4d3bebc0555d5f1fb9480bb7eff0dee1b335 Mon Sep 17 00:00:00 2001
From: James Coonradt <gamax92@aol.com>
Date: Tue, 19 Sep 2017 14:08:12 -0600
Subject: [PATCH] user32: Fix FlashWindowEx message and return value
---
dlls/user32/tests/win.c | 52 ++++++++++++++++++++++++++++++-------------------
dlls/user32/win.c | 8 +++++---
2 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/dlls/user32/tests/win.c b/dlls/user32/tests/win.c
index 9b8d16adc5..52a8804ae5 100644
--- a/dlls/user32/tests/win.c
+++ b/dlls/user32/tests/win.c
@@ -77,6 +77,7 @@ static HWND hwndMessage;
static HWND hwndMain, hwndMain2;
static HHOOK hhook;
static BOOL app_activated, app_deactivated;
+static BOOL nc_activated, nc_activate_w;
static const char* szAWRClass = "Winsize";
static HMENU hmenu;
@@ -863,6 +864,10 @@ static LRESULT WINAPI main_window_procA(HWND hwnd, UINT msg, WPARAM wparam, LPAR
if (wparam) app_activated = TRUE;
else app_deactivated = TRUE;
break;
+ case WM_NCACTIVATE:
+ nc_activated = TRUE;
+ nc_activate_w = (wparam != 0);
+ break;
}
return DefWindowProcA(hwnd, msg, wparam, lparam);
@@ -8279,11 +8284,12 @@ static void test_FlashWindow(void)
"FlashWindow returned with %d\n", GetLastError() );
}
-static void test_FlashWindowEx(void)
+static void test_FlashWindowEx(HWND hwndMain)
{
HWND hwnd;
FLASHWINFO finfo;
- BOOL prev, ret;
+ BOOL expected, ret;
+ int i, flags;
if (!pFlashWindowEx)
{
@@ -8314,7 +8320,7 @@ static void test_FlashWindowEx(void)
SetLastError(0xdeadbeef);
ret = pFlashWindowEx(&finfo);
- todo_wine ok(!ret, "previous window state should not be active\n");
+ ok(!ret, "previous window state should not be active\n");
finfo.cbSize = sizeof(FLASHWINFO) - 1;
SetLastError(0xdeadbeef);
@@ -8343,29 +8349,35 @@ static void test_FlashWindowEx(void)
ok(finfo.uCount == 3, "FlashWindowEx modified uCount to %x\n", finfo.uCount);
ok(finfo.dwTimeout == 200, "FlashWindowEx modified dwTimeout to %x\n", finfo.dwTimeout);
- hwnd = CreateWindowExA( 0, "MainWindowClass", "FlashWindow", WS_VISIBLE | WS_POPUPWINDOW,
- 0, 0, 0, 0, 0, 0, 0, NULL );
- ok( hwnd != 0, "CreateWindowExA error %d\n", GetLastError() );
- finfo.hwnd = hwnd;
+ finfo.hwnd = hwndMain;
+ finfo.uCount = 1;
+ finfo.dwTimeout = 1;
SetLastError(0xdeadbeef);
ret = pFlashWindowEx(NULL);
ok(!ret && GetLastError() == ERROR_NOACCESS,
"FlashWindowEx returned with %d\n", GetLastError());
- SetLastError(0xdeadbeef);
- prev = pFlashWindowEx(&finfo);
-
- ok(finfo.cbSize == sizeof(FLASHWINFO), "FlashWindowEx modified cdSize to %x\n", finfo.cbSize);
- ok(finfo.hwnd == hwnd, "FlashWindowEx modified hwnd to %p\n", finfo.hwnd);
- ok(finfo.dwFlags == FLASHW_TIMER, "FlashWindowEx modified dwFlags to %x\n", finfo.dwFlags);
- ok(finfo.uCount == 3, "FlashWindowEx modified uCount to %x\n", finfo.uCount);
- ok(finfo.dwTimeout == 200, "FlashWindowEx modified dwTimeout to %x\n", finfo.dwTimeout);
+ for (i = 0; i < 2; i++)
+ {
+ ShowWindow(finfo.hwnd, i ? SW_HIDE : SW_SHOW);
+ expected = !i;
- finfo.dwFlags = FLASHW_STOP;
- SetLastError(0xdeadbeef);
- ret = pFlashWindowEx(&finfo);
- ok(prev != ret, "previous window state should be different\n");
+ for (flags = 0; flags < 16; flags++)
+ {
+ nc_activated = nc_activate_w = FALSE;
+ finfo.dwFlags = flags;
+ ret = pFlashWindowEx(&finfo) != 0;
+ Sleep(50);
+ flush_events(TRUE);
+ ok(nc_activated, "didn't get expected WM_NCACTIVATE message\n");
+ ok(expected == nc_activate_w, "expected WM_NCACTIVE message with wParam == %u, got %u\n", expected, nc_activate_w);
+ if (finfo.dwFlags & FLASHW_CAPTION)
+ ok(ret == TRUE, "expected FlashWindowEx to return TRUE, got %s\n", ret ? "TRUE" : "FALSE");
+ else
+ ok(ret == expected, "expected FlashWindowEx to return %s, got %s\n", expected ? "TRUE" : "FALSE", ret ? "TRUE" : "FALSE");
+ }
+ }
DestroyWindow( hwnd );
user image Sebastian Lackner
20 Sep. 17

This DestroyWindow call is no longer necessary btw.

}
@@ -10127,7 +10139,7 @@ START_TEST(win)
test_capture_4();
test_rtl_layout();
test_FlashWindow();
- test_FlashWindowEx();
+ test_FlashWindowEx(hwndMain);
test_CreateWindow();
test_parent_owner();
diff --git a/dlls/user32/win.c b/dlls/user32/win.c
index 3042a560ce..563e275ec1 100644
--- a/dlls/user32/win.c
+++ b/dlls/user32/win.c
@@ -3545,13 +3545,15 @@ BOOL WINAPI FlashWindowEx( PFLASHWINFO pfinfo )
if (!wndPtr || wndPtr == WND_OTHER_PROCESS || wndPtr == WND_DESKTOP) return FALSE;
hwnd = wndPtr->obj.handle; /* make it a full handle */
- if (pfinfo->dwFlags) wparam = !(wndPtr->flags & WIN_NCACTIVATED);
- else wparam = (hwnd == GetForegroundWindow());
+ wparam = (wndPtr->flags & WIN_NCACTIVATED) != 0;
WIN_ReleasePtr( wndPtr );
SendMessageW( hwnd, WM_NCACTIVATE, wparam, 0 );
USER_Driver->pFlashWindowEx( pfinfo );
- return wparam;
+ if (pfinfo->dwFlags & FLASHW_CAPTION)
+ return TRUE;
+ else
+ return wparam;
}
}
--
2.11.0