[PATCH 0/2] MR1886: Fixing paste errors on edit control in Win10+.
This series tries to fix WM_PASTE tests error on edit control (both user32 & comctl32). The test failures where systematic on a local Win10 for comctl32 (yet very rare on user32, perhaps one in 20 runs). Since the failures are not systematic, it ought to be a synchronisation / timing issue. Flushing the msg queue before starting the paste test cycle seems to fix the issue. - 0 occurence on comctl32 and user32 in 50 run (local VM) - 0 occurence on Testbot (comctl32) in 1 run. It's hard to be fully conclusive about bug resolution, but it's at least a step into the right direction. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1886
From: Eric Pouech <eric.pouech(a)gmail.com> The edit control should process all events in queue to be fully initialized (otherwise the WM_PASTE will bypass some pending messages). Wine-bug: https://bugs.winehq.org/show_bug.cgi?id=53276 Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/user32/tests/edit.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index 2afbbd5bea4..dd689c80f84 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -38,6 +38,24 @@ struct edit_notify { static struct edit_notify notifications; +/* try to make sure pending X events have been processed before continuing */ +static void flush_events(void) +{ + MSG msg; + int diff = 200; + int min_timeout = 100; + DWORD time = GetTickCount() + diff; + + while (diff > 0) + { + if (MsgWaitForMultipleObjects(0, NULL, FALSE, min_timeout, QS_ALLINPUT) == WAIT_TIMEOUT) + break; + while (PeekMessageA(&msg, 0, 0, 0, PM_REMOVE)) + DispatchMessageA(&msg); + diff = time - GetTickCount(); + } +} + static INT_PTR CALLBACK multi_edit_dialog_proc(HWND hdlg, UINT msg, WPARAM wparam, LPARAM lparam) { static int num_ok_commands = 0; @@ -3204,6 +3222,8 @@ static void test_paste(void) r = CloseClipboard(); ok(r == TRUE, "expected %d, got %d\n", TRUE, r); + flush_events(); + /* Paste single line */ SendMessageA(hEdit, WM_SETTEXT, 0, (LPARAM)""); r = SendMessageA(hEdit, WM_PASTE, 0, 0); @@ -3227,12 +3247,16 @@ static void test_paste(void) r = CloseClipboard(); ok(r == TRUE, "expected %d, got %d\n", TRUE, r); + flush_events(); + /* Paste multiline text in singleline edit - should be cut */ SendMessageA(hEdit, WM_SETTEXT, 0, (LPARAM)""); r = SendMessageA(hEdit, WM_PASTE, 0, 0); len = SendMessageA(hEdit, WM_GETTEXTLENGTH, 0, 0); ok(strlen("first line") == len, "got %d\n", len); + flush_events(); + /* Paste multiline text in multiline edit */ SendMessageA(hMultilineEdit, WM_SETTEXT, 0, (LPARAM)""); r = SendMessageA(hMultilineEdit, WM_PASTE, 0, 0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1886
From: Eric Pouech <eric.pouech(a)gmail.com> The edit control should process all events in queue to be fully initialized (otherwise the WM_PASTE will bypass some pending messages). Wine-bug: https://bugs.winehq.org/show_bug.cgi?id=53276 Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- dlls/comctl32/tests/edit.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index 91d16e044a0..19c77aeb4fb 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -3208,6 +3208,8 @@ static void test_paste(void) r = CloseClipboard(); ok(r == TRUE, "expected %d, got %d\n", TRUE, r); + flush_events(); + /* Paste single line */ SendMessageA(hEdit, WM_SETTEXT, 0, (LPARAM)""); r = SendMessageA(hEdit, WM_PASTE, 0, 0); @@ -3231,12 +3233,16 @@ static void test_paste(void) r = CloseClipboard(); ok(r == TRUE, "expected %d, got %d\n", TRUE, r); + flush_events(); + /* Paste multiline text in singleline edit - should be cut */ SendMessageA(hEdit, WM_SETTEXT, 0, (LPARAM)""); r = SendMessageA(hEdit, WM_PASTE, 0, 0); len = SendMessageA(hEdit, WM_GETTEXTLENGTH, 0, 0); ok(strlen("first line") == len, "got %d\n", len); + flush_events(); + /* Paste multiline text in multiline edit */ SendMessageA(hMultilineEdit, WM_SETTEXT, 0, (LPARAM)""); r = SendMessageA(hMultilineEdit, WM_PASTE, 0, 0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1886
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
hEdit = create_editcontrol(ES_AUTOHSCROLL | ES_AUTOVSCROLL, 0); hMultilineEdit = create_editcontrol(ES_AUTOHSCROLL | ES_AUTOVSCROLL | ES_MULTILINE, 0);
Is a flush_events() call after edit window creation enough? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1886#note_20150
On Mon Dec 26 03:09:17 2022 +0000, Zhiyi Zhang wrote:
Is a flush_events() call after edit window creation enough? Unfortunately, no. I thought about it and tested it when I wrote the patch and it isn't always sufficient. Retested, still same result.
Adding some more traces about the messages I get (and messages sequence looks pretty consistent across runs): - after window creation: two WM_DWMNCRENDERINGCHANGED messages (wp=1, lp=0) (one for each created edit window) - after the first write to the clipboard: WM_TIMER (wp=1, lp=0) (message to window of class 'MSCTFIME UI' and title 'MSCTFIME' - none after the second write to the clipboard (and afterwards) Note: in early testing of the cause of the failures, I noticed that adding some Sleep() calls after writing to the clipboard reduced the number of occurence of failures (without eradicating it). A possible interpretation: the windows implementation doesn't paste clipboard content if the input message queue isn't empty. But I couldn't find doc/report by googling for this interpretation. Does it make sense to you? (if interpretation is correct, then the patch reduces the likehood of having a msg in input queue, without fully eradicating it) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1886#note_20153
On Mon Dec 26 09:00:52 2022 +0000, eric pouech wrote:
Unfortunately, no. I thought about it and tested it when I wrote the patch and it isn't always sufficient. Retested, still same result. Adding some more traces about the messages I get (and messages sequence looks pretty consistent across runs): - after window creation: two WM_DWMNCRENDERINGCHANGED messages (wp=1, lp=0) (one for each created edit window) - after the first write to the clipboard: WM_TIMER (wp=1, lp=0) (message to window of class 'MSCTFIME UI' and title 'MSCTFIME' - none after the second write to the clipboard (and afterwards) Note: in early testing of the cause of the failures, I noticed that adding some Sleep() calls after writing to the clipboard reduced the number of occurence of failures (without eradicating it). A possible interpretation: the windows implementation doesn't paste clipboard content if the input message queue isn't empty. But I couldn't find doc/report by googling for this interpretation. Does it make sense to you? (if interpretation is correct, then the patch reduces the likehood of having a msg in input queue, without fully eradicating it) actually, this doesn't look to be the case. After several different testings methods, adding some busy-loop after closing the clipboard makes the test pass (without any event flushing involved). it looks like the data in the clipboard are not immediately available after CloseClipboard() returns... I don't like the Sleep(xx) in-between, and couldn't think of a way to synchronize. so replacing SendMessageA(h???, WM_PASTE, 0, 0); with something like
for (retry = 0; retry < 3; retry++)
{
SendMessageA(h???, WM_PASTE, 0, 0);
if (SendMessageA(h???, WM_GETTEXTLENGTH, 0, 0)) break;
Sleep(20);
}
could do the trick -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1886#note_20155
participants (3)
-
Eric Pouech -
eric pouech (＠epo)
-
Zhiyi Zhang (＠zhiyi)