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@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.
-
Nikolay Sivov