[v2 PATCH] user32/msgbox: Support WM_COPY Message
Nikolay Sivov
bunglehead at gmail.com
Tue Nov 8 05:05:56 CST 2016
On 08.11.2016 13:44, Alistair Leslie-Hughes wrote:
> Fixes: https://bugs.winehq.org/show_bug.cgi?id=17205
>
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair at hotmail.com>
> ---
> dlls/user32/msgbox.c | 86 +++++++++++++++++++--
> dlls/user32/tests/dialog.c | 187 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 267 insertions(+), 6 deletions(-)
>
> diff --git a/dlls/user32/msgbox.c b/dlls/user32/msgbox.c
> index 2ba98c9..4735b8a 100644
> --- a/dlls/user32/msgbox.c
> +++ b/dlls/user32/msgbox.c
> @@ -44,6 +44,11 @@ struct ThreadWindows
> HWND *handles;
> };
>
> +/* Index the order the buttons need to appear to an ID* constant */
> +static const int buttonOrder[10] = { IDYES, IDNO, IDOK, IDABORT, IDRETRY,
> + IDCANCEL, IDIGNORE, IDTRYAGAIN,
> + IDCONTINUE, IDHELP };
> +
> static BOOL CALLBACK MSGBOX_EnumProc(HWND hwnd, LPARAM lParam)
> {
> struct ThreadWindows *threadWindows = (struct ThreadWindows *)lParam;
> @@ -77,11 +82,6 @@ static void MSGBOX_OnInit(HWND hwnd, LPMSGBOXPARAMSW lpmb)
> WCHAR *buffer = NULL;
> const WCHAR *ptr;
>
> - /* Index the order the buttons need to appear to an ID* constant */
> - static const int buttonOrder[10] = { IDYES, IDNO, IDOK, IDABORT, IDRETRY,
> - IDCANCEL, IDIGNORE, IDTRYAGAIN,
> - IDCONTINUE, IDHELP };
> -
> nclm.cbSize = sizeof(nclm);
> SystemParametersInfoW (SPI_GETNONCLIENTMETRICS, 0, &nclm, 0);
>
> @@ -319,6 +319,77 @@ static void MSGBOX_OnInit(HWND hwnd, LPMSGBOXPARAMSW lpmb)
> HeapFree( GetProcessHeap(), 0, buffer );
> }
>
> +static void MSGBOX_CopyToClipboard( HWND hwnd )
> +{
> + int i;
> + static const WCHAR line[] = {'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-',
> + '-','-','-','-','-','-','-','-','\r','\n', 0};
> + static const WCHAR crlf[] = {'\r','\n', 0};
> + static const WCHAR spaces[] = {' ',' ',' ', 0};
> + int lenTitle = GetWindowTextLengthW(hwnd) + 1;
> + int lenMsg = GetWindowTextLengthW(GetDlgItem(hwnd, MSGBOX_IDTEXT)) + 1;
> +
> + /*
> + ---------------------------
> + Dialog Title
> + ---------------------------
> + Dialog Message
> + ---------------------------
> + Button(s) Text. OK
> + ---------------------------
> + */
> + int len = ((sizeof(crlf) * 3) + (sizeof(line) * 4) + lenTitle + lenMsg) * sizeof(WCHAR);
This will allocate more than you need.
> + WCHAR *text = HeapAlloc( GetProcessHeap(), 0, len);
Maybe return right here if allocation failed? Also please use consistent
formatting regarding spaces.
> + if(text)
> + {
> + lstrcpyW(text, line);
> + if (GetWindowTextW(hwnd, text + lstrlenW(text), lenTitle))
> + {
> + HGLOBAL hMem;
> + WCHAR *data;
> +
> + lstrcatW(text, crlf);
> + lstrcatW(text, line);
> + GetWindowTextW(GetDlgItem(hwnd, MSGBOX_IDTEXT), text + lstrlenW(text), lenMsg);
There is GetDlgItemText() for that.
> + lstrcatW(text, crlf);
> + lstrcatW(text, line);
> +
> + for (i = 0; i < (sizeof(buttonOrder) / sizeof(buttonOrder[0])); i++)
> + {
> + HWND hItem = GetDlgItem(hwnd, buttonOrder[i]);
> + if (GetWindowLongW(hItem, GWL_STYLE) & WS_VISIBLE)
> + {
Does IsWindowVisible() work here?
> + WCHAR buffer[1024] = {0};
This looks excessive for a button text string.
>
> case WM_COMMAND:
> switch (LOWORD(wParam))
> diff --git a/dlls/user32/tests/dialog.c b/dlls/user32/tests/dialog.c
> index a37c678..0bb7dca 100644
> --- a/dlls/user32/tests/dialog.c
> +++ b/dlls/user32/tests/dialog.c
> @@ -40,6 +40,7 @@
> #include "winbase.h"
> #include "wingdi.h"
> #include "winuser.h"
> +#include "winnls.h"
>
> #define MAXHWNDS 1024
> static HWND hwnd [MAXHWNDS];
> @@ -1392,6 +1393,187 @@ static void test_MessageBoxFontTest(void)
> DestroyWindow(hDlg);
> }
>
> +static const char msgbox_title[] = "%5!z9ZXw*ia;57n/FGl.bCH,Su\"mfKN;foCqAU\'j6AmoJgAc_D:Z0A\'E6PF_O/w";
What's the idea behind such title? If you're trying to create a unique
window title, I don't think it has to be that unique.
> +static WCHAR expectedOK[] =
> +{
> +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n',
> +'%','5','!','z','9','Z','X','w','*','i','a',';','5','7','n','/','F','G','l','.','b','C','H',',','S','u','"','m','f',
> +'K','N',';','f','o','C','q','A','U','\'','j','6','A','m','o','J','g','A','c','_','D',':','Z','0','A','\'','E','6','P',
> +'F','_','O','/','w','\r','\n',
> +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n',
> +'M','e','s','s','a','g','e','\r','\n',
> +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n',
> +'O','K',' ',' ',' ','\r','\n',
> +'-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','-','\r','\n', 0
> +};
Instead of expected strings you can as well duplicate your
implementation logic and generate expected result. This will help
testing with localized dialogs.
> +
> +DWORD WINAPI WorkerThread(void *param)
Please please use more specific name.
> + {
> + succeeded = lstrcmpW(expected, text) == 0;
> + if(!succeeded)
> + {
> + ok(0, "%s\n", wine_dbgstr_w(text));
> + ok(0, "%s\n", wine_dbgstr_w(expected));
> + }
> + }
Could this simply be ok(!lstrcmpW(), "..."); ?
> +
> +static void test_MessageBox_WM_COPY_Test(void)
> +{
> + DWORD tid = 0;
> +
> + non_english = (PRIMARYLANGID(GetUserDefaultLangID()) != LANG_ENGLISH);
> + trace("non_english %d\n", non_english);
> +
> + CreateThread(NULL, 0, WorkerThread, &expectedOK, 0, &tid);
> + MessageBoxA(NULL, "Message", msgbox_title, MB_OK);
> +
> + CreateThread(NULL, 0, WorkerThread, &expectedOkCancel, 0, &tid);
> + MessageBoxA(NULL, "Message", msgbox_title, MB_OKCANCEL);
> +
> + CreateThread(NULL, 0, WorkerThread, &expectedAbortRetryIgnore, 0, &tid);
> + MessageBoxA(NULL, "Message", msgbox_title, MB_ABORTRETRYIGNORE);
> +
> + CreateThread(NULL, 0, WorkerThread, &expectedYesNo, 0, &tid);
> + MessageBoxA(NULL, "Message", msgbox_title, MB_YESNO);
> +
> + CreateThread(NULL, 0, WorkerThread, &expectedYesNoCancel, 0, &tid);
> + MessageBoxA(NULL, "Message", msgbox_title, MB_YESNOCANCEL);
> +
> + CreateThread(NULL, 0, WorkerThread, &expectedRetryCancel, 0, &tid);
> + MessageBoxA(NULL, "Message", msgbox_title, MB_RETRYCANCEL);
> +
> + CreateThread(NULL, 0, WorkerThread, &expectedCancelTryContinue, 0, &tid);
> + MessageBoxA(NULL, "Message", msgbox_title, MB_CANCELTRYCONTINUE);
> +}
This could be a loop over possible button values.
More information about the wine-devel
mailing list