[PATCH 1/3] comctl32/pager: Support toolbar notification conversion.
Fix BibleWorks 10 not displaying toolbar. Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/comctl32/pager.c | 96 ++++++++ dlls/comctl32/tests/pager.c | 431 ++++++++++++++++++++++++++++++++++++ 2 files changed, 527 insertions(+) diff --git a/dlls/comctl32/pager.c b/dlls/comctl32/pager.c index de63cad717..0367108db2 100644 --- a/dlls/comctl32/pager.c +++ b/dlls/comctl32/pager.c @@ -91,6 +91,17 @@ typedef struct #define INITIAL_DELAY 500 #define REPEAT_DELAY 50 +/* Text field conversion behavior flags for PAGER_SendConvertedNotify() */ +typedef enum +{ + /* Convert ANSI text from parent back to Unicode for pager */ + FROM_ANSI = 0x01, + /* Convert Unicode text to ANSI for parent before sending. If not set, do nothing */ + TO_ANSI = 0x02, + /* Send empty text to parent if text is NULL, original text pointer still remain NULL */ + SET_NULL_EMPTY = 0x04 +} ConversionFlag; + static void PAGER_GetButtonRects(const PAGER_INFO* infoPtr, RECT* prcTopLeft, RECT* prcBottomRight, BOOL bClientCoords) { @@ -1020,6 +1031,89 @@ static LRESULT PAGER_NotifyFormat(PAGER_INFO *infoPtr, INT command) } } +static UINT PAGER_GetAnsiNtfCode(UINT code) +{ + switch (code) + { + /* Toolbar */ + case TBN_GETBUTTONINFOW: return TBN_GETBUTTONINFOA; + case TBN_GETINFOTIPW: return TBN_GETINFOTIPA; + } + return code; +} + +static LRESULT PAGER_SendConvertedNotify(PAGER_INFO *infoPtr, NMHDR *hdr, WCHAR **text, INT *textMax, ConversionFlag flags) +{ + WCHAR emptyW[] = {0}; + WCHAR *oldText; + INT oldTextMax; + void *sendBuffer = NULL; + void *receiveBuffer = NULL; + INT bufferSize; + LRESULT ret = 0; + + oldText = *text; + oldTextMax = textMax ? *textMax : 0; + + hdr->code = PAGER_GetAnsiNtfCode(hdr->code); + + if (oldTextMax < 0) goto done; + + if (!*text && flags & SET_NULL_EMPTY) + *text = emptyW; + + if (*text && flags & TO_ANSI) + { + bufferSize = WideCharToMultiByte(CP_ACP, 0, *text, -1, 0, 0, NULL, FALSE); + bufferSize = max(bufferSize, oldTextMax); + sendBuffer = heap_alloc(bufferSize); + if (!sendBuffer) goto done; + WideCharToMultiByte(CP_ACP, 0, *text, -1, sendBuffer, bufferSize, NULL, FALSE); + *text = sendBuffer; + } + + ret = SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); + + if (*text && flags & FROM_ANSI) + { + bufferSize = lstrlenA((CHAR *)*text); + receiveBuffer = heap_alloc(bufferSize + 1); + if (!receiveBuffer) goto done; + memcpy(receiveBuffer, *text, bufferSize + 1); + if (!oldText) goto done; + MultiByteToWideChar(CP_ACP, 0, receiveBuffer, -1, oldText, oldTextMax); + } + +done: + heap_free(sendBuffer); + heap_free(receiveBuffer); + *text = oldText; + return ret; +} + +static LRESULT PAGER_Notify(PAGER_INFO *infoPtr, NMHDR *hdr) +{ + if (infoPtr->bUnicode) return SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); + + switch (hdr->code) + { + /* Toolbar */ + case TBN_GETBUTTONINFOW: + { + NMTOOLBARW *nmtb = (NMTOOLBARW *)hdr; + return PAGER_SendConvertedNotify(infoPtr, hdr, &nmtb->pszText, &nmtb->cchText, + SET_NULL_EMPTY | TO_ANSI | FROM_ANSI); + } + case TBN_GETINFOTIPW: + { + NMTBGETINFOTIPW *nmtbgit = (NMTBGETINFOTIPW *)hdr; + return PAGER_SendConvertedNotify(infoPtr, hdr, &nmtbgit->pszText, &nmtbgit->cchTextMax, FROM_ANSI); + } + } + /* Other notifications or notifications above that don't have text, thus no need to convert */ + return SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); +} + static LRESULT WINAPI PAGER_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { @@ -1115,6 +1209,8 @@ PAGER_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) return PAGER_NotifyFormat (infoPtr, lParam); case WM_NOTIFY: + return PAGER_Notify (infoPtr, (NMHDR *)lParam); + case WM_COMMAND: return SendMessageW (infoPtr->hwndNotify, uMsg, wParam, lParam); diff --git a/dlls/comctl32/tests/pager.c b/dlls/comctl32/tests/pager.c index a1f6683a57..46f2eb0acb 100644 --- a/dlls/comctl32/tests/pager.c +++ b/dlls/comctl32/tests/pager.c @@ -30,10 +30,91 @@ static HWND parent_wnd, child1_wnd, child2_wnd; static INT notify_format; static BOOL notify_query_received; +static WCHAR test_w[] = {'t', 'e', 's', 't', 0}; +static CHAR test_a[] = {'t', 'e', 's', 't', 0}; +static WCHAR t_w[] = {'t', 0}; +static CHAR large_a[] = "You should have received a copy of the GNU Lesser General Public License along with this ..."; +static WCHAR buffer[64]; +static WCHAR buffer2[64]; + +/* Text field conversion behavior flags for PAGER_SendConvertedNotify() */ +typedef enum +{ + /* Convert ANSI text from parent back to Unicode for pager */ + FROM_ANSI = 0x01, + /* Convert Unicode text to ANSI for parent before sending. If not set, do nothing */ + TO_ANSI = 0x02, + /* Send empty text to parent if text is NULL, original text pointer still remain NULL */ + SET_NULL_EMPTY = 0x04 +} ConversionFlag; + +static struct notify_test_data +{ + UINT unicode; + UINT ansi; + UINT_PTR id_from; + HWND hwnd_from; + INT text_size; + /* Whether parent receive notification */ + BOOL received; + UINT sub_test_id; + /* Text field conversion behavior flags */ + ConversionFlag flags; +} notify_test_data; #define CHILD1_ID 1 #define CHILD2_ID 2 +#define expect_eq_int(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " 0x%08x, got 0x%08x\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_long(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " 0x%08lx, got 0x%08lx\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_pointer(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " %p, got %p\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_text_a(actual, expected) \ + do \ + { \ + ok(!lstrcmpA((actual), (expected)), "Code:0x%08x Subtest:0x%08x Expect " #actual " %s, got %s\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_text_w(actual, expected) \ + do \ + { \ + ok(!lstrcmpW((actual), (expected)), "Code:0x%08x Subtest:0x%08x Expect " #actual " %s, got %s\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, wine_dbgstr_w((expected)), \ + wine_dbgstr_w((actual))); \ + } while (0) + +#define expect_text_empty(actual) \ + do \ + { \ + ok((actual) && !(actual)[0], "Code:0x%08x Subtest:0x%08x Expect " #actual " empty\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id); \ + } while (0) + +#define expect_null(actual) \ + do \ + { \ + ok(!(actual), "Code:0x%08x Subtest:0x%08x Expect " #actual " NULL, got %p\n", notify_test_data.unicode, \ + notify_test_data.sub_test_id, (actual)); \ + } while (0) + static BOOL (WINAPI *pInitCommonControlsEx)(const INITCOMMONCONTROLSEX*); static BOOL (WINAPI *pSetWindowSubclass)(HWND, SUBCLASSPROC, UINT_PTR, DWORD_PTR); @@ -419,6 +500,355 @@ static void test_wm_notifyformat(void) UnregisterClassW(class_w, GetModuleHandleW(NULL)); } +static void wm_notify_text_handler(CHAR **text, INT *textMax) +{ + expect_eq_int(*textMax, notify_test_data.text_size); + switch (notify_test_data.sub_test_id) + { + case 1: + if (notify_test_data.flags & SET_NULL_EMPTY) + expect_text_empty(*text); + else + expect_null(*text); + break; + case 2: + { + INT length = lstrlenA(*text); + ok(length == 0 + || broken(length == 3) /* 2003 */ + || broken(length == 1) /* <= Win7 */, + "Wrong text length\n"); + break; + } + case 3: + if (notify_test_data.flags & TO_ANSI) + { + expect_eq_text_a(*text, test_a); + ok(*text != (CHAR *)buffer, "Expect using a new buffer\n"); + } + else + { + expect_eq_text_w((WCHAR *)*text, test_w); + expect_eq_pointer(*text, (CHAR *)buffer); + } + break; + case 4: + expect_text_empty(*text); + memcpy(*text, test_a, sizeof(test_a)); + break; + case 5: + expect_text_empty(*text); + *textMax = 1; + break; + case 6: + expect_text_empty(*text); + *text = test_a; + break; + case 7: + *text = test_a; + break; + case 8: + expect_text_empty(*text); + *text = large_a; + break; + } +} + +static LRESULT WINAPI test_notify_proc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) +{ + static const WCHAR test[] = {'t', 'e', 's', 't', 0}; + switch (message) + { + case WM_NOTIFY: + { + NMHDR *hdr = (NMHDR *)lParam; + + /* Not notifications we want to test */ + if (!notify_test_data.unicode) break; + ok(!notify_test_data.received, "Extra notification received\n"); + + expect_eq_long(wParam, notify_test_data.id_from); + expect_eq_int(hdr->code, notify_test_data.ansi); + expect_eq_long(hdr->idFrom, notify_test_data.id_from); + expect_eq_pointer(hdr->hwndFrom, notify_test_data.hwnd_from); + + if (hdr->code != notify_test_data.ansi) + { + skip("Notification code mismatch, skipping lParam check\n"); + return 0; + } + switch (hdr->code) + { + /* Toolbar */ + /* No conversion for TBN_SAVE and TBN_RESTORE */ + case TBN_SAVE: + { + NMTBSAVE *nmtbs = (NMTBSAVE *)hdr; + switch (notify_test_data.sub_test_id) + { + case 0: + expect_eq_text_w((WCHAR *)nmtbs->tbButton.iString, test_w); + break; + case 1: + nmtbs->tbButton.iString = (INT_PTR)test_a; + break; + } + break; + } + case TBN_RESTORE: + { + NMTBRESTORE *nmtbr = (NMTBRESTORE *)hdr; + switch (notify_test_data.sub_test_id) + { + case 0: + expect_eq_text_w((WCHAR *)nmtbr->tbButton.iString, test_w); + break; + case 1: + nmtbr->tbButton.iString = (INT_PTR)test_a; + break; + } + break; + } + case TBN_GETBUTTONINFOA: + { + NMTOOLBARA *nmtb = (NMTOOLBARA *)hdr; + wm_notify_text_handler(&nmtb->pszText, &nmtb->cchText); + break; + } + /* TBN_GETDISPINFOW doesn't get converted to TBN_GETDISPINFOA */ + case TBN_GETDISPINFOW: + { + NMTBDISPINFOW *nmtbdi = (NMTBDISPINFOW *)hdr; + expect_eq_text_w(nmtbdi->pszText, test_w); + expect_eq_int(nmtbdi->cchText, ARRAY_SIZE(buffer)); + memcpy(nmtbdi->pszText, test_a, sizeof(test_a)); + break; + } + case TBN_GETINFOTIPA: + { + NMTBGETINFOTIPA *nmtbgit = (NMTBGETINFOTIPA *)hdr; + wm_notify_text_handler(&nmtbgit->pszText, &nmtbgit->cchTextMax); + break; + } + default: + ok(0, "Unexpected message 0x%08x\n", hdr->code); + } + notify_test_data.received = TRUE; + ok(!lstrcmpA(test_a, "test"), "test_a got modified\n"); + ok(!lstrcmpW(test_w, test), "test_w got modified\n"); + return 0; + } + case WM_NOTIFYFORMAT: + if (lParam == NF_QUERY) return NFR_ANSI; + break; + } + return DefWindowProcA(hwnd, message, wParam, lParam); +} + +static BOOL register_test_notify_class(void) +{ + WNDCLASSA cls = {0}; + + cls.lpfnWndProc = test_notify_proc; + cls.hInstance = GetModuleHandleA(NULL); + cls.lpszClassName = "Pager notify class"; + return RegisterClassA(&cls); +} + +static void send_notify_(HWND pager, UINT unicode, UINT ansi, LPARAM lParam, BOOL code_change, BOOL received) +{ + NMHDR *hdr = (NMHDR *)lParam; + + notify_test_data.unicode = unicode; + notify_test_data.id_from = 1; + notify_test_data.hwnd_from = child1_wnd; + notify_test_data.ansi = ansi; + notify_test_data.received = FALSE; + + hdr->code = unicode; + hdr->idFrom = 1; + hdr->hwndFrom = child1_wnd; + + SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam); + expect_eq_int(notify_test_data.received, received); + expect_eq_int(hdr->code, code_change ? notify_test_data.ansi : notify_test_data.unicode); +} + +#define send_notify(a, b, c, d, e) send_notify_(pager, a, b, c, d, e) + +/* Send notify to test text field conversion. In parent proc wm_notify_text_handler() handles these messages */ +static void test_wm_notify_text_helper_(HWND pager, void *ptr, size_t size, WCHAR **text, INT *textMax, + UINT code_unicode, UINT code_ansi, ConversionFlag flags) +{ + notify_test_data.flags = flags; + + /* NULL text */ + notify_test_data.sub_test_id = 1; + memset(ptr, 0, size); + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_null(*text); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Zero text max size */ + notify_test_data.sub_test_id = 2; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + /* Got *text null on Windows <= Win7 */ + ok(broken(!*text) || (!lstrlenW(*text) && *text == buffer), "Wrong *text\n"); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification */ + notify_test_data.sub_test_id = 3; + memset(ptr, 0, size); + memcpy(buffer, test_w, sizeof(test_w)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + if (!(flags & TO_ANSI) && flags & FROM_ANSI) + expect_eq_text_w(*text, t_w); + else + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent write to buffer */ + notify_test_data.sub_test_id = 4; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent set buffer size to one */ + notify_test_data.sub_test_id = 5; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, 1); + + /* Valid notification. Parent replace buffer */ + notify_test_data.sub_test_id = 6; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* NULL text. Parent replace buffer */ + notify_test_data.sub_test_id = 7; + memset(ptr, 0, size); + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_null(*text); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent replace buffer with a pointer pointing to a string larger than old buffer can store */ + notify_test_data.sub_test_id = 8; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + memset(buffer2, 0, sizeof(buffer2)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + MultiByteToWideChar(CP_ACP, 0, large_a, -1, buffer2, ARRAY_SIZE(buffer2)); + ok(!memcmp(*text, buffer2, sizeof(buffer2)), "Expect memory content to be the same\n"); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); +} + +#define test_wm_notify_text_helper(a, b, c, d, e, f, g) test_wm_notify_text_helper_(pager, a, b, c, d, e, f, g) + +static void test_wm_notify_toolbar(HWND pager) +{ + NMTBRESTORE nmtbr; + NMTBSAVE nmtbs; + NMTOOLBARW nmtb; + NMTBDISPINFOW nmtbdi; + NMTBGETINFOTIPW nmtbgit; + + /* No conversion for TBN_SAVE */ + notify_test_data.sub_test_id = 0; + memset(&nmtbs, 0, sizeof(nmtbs)); + nmtbs.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_SAVE, TBN_SAVE, (LPARAM)&nmtbs, TRUE, TRUE); + expect_eq_text_w((WCHAR *)nmtbs.tbButton.iString, test_w); + + notify_test_data.sub_test_id = 1; + memset(&nmtbs, 0, sizeof(nmtbs)); + nmtbs.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_SAVE, TBN_SAVE, (LPARAM)&nmtbs, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbs.tbButton.iString, test_a); + + /* No conversion for TBN_RESTORE */ + notify_test_data.sub_test_id = 0; + memset(&nmtbr, 0, sizeof(nmtbr)); + nmtbr.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_RESTORE, TBN_RESTORE, (LPARAM)&nmtbr, TRUE, TRUE); + expect_eq_text_w((WCHAR *)nmtbr.tbButton.iString, test_w); + + notify_test_data.sub_test_id = 1; + memset(&nmtbr, 0, sizeof(nmtbr)); + nmtbr.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_RESTORE, TBN_RESTORE, (LPARAM)&nmtbr, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbr.tbButton.iString, test_a); + + /* TBN_GETDISPINFOW doesn't get unconverted */ + memset(&nmtbdi, 0, sizeof(nmtbdi)); + memcpy(buffer, test_w, sizeof(test_w)); + nmtbdi.dwMask = TBNF_TEXT; + nmtbdi.pszText = buffer; + nmtbdi.cchText = ARRAY_SIZE(buffer); + notify_test_data.text_size = nmtbdi.cchText; + send_notify(TBN_GETDISPINFOW, TBN_GETDISPINFOW, (LPARAM)&nmtbdi, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbdi.pszText, test_a); + + test_wm_notify_text_helper(&nmtb, sizeof(nmtb), &nmtb.pszText, &nmtb.cchText, TBN_GETBUTTONINFOW, + TBN_GETBUTTONINFOA, SET_NULL_EMPTY | TO_ANSI); + test_wm_notify_text_helper(&nmtbgit, sizeof(nmtbgit), &nmtbgit.pszText, &nmtbgit.cchTextMax, TBN_GETINFOTIPW, + TBN_GETINFOTIPA, FROM_ANSI); +} + +static void test_wm_notify(void) +{ + static const CHAR *class = "Pager notify class"; + HWND parent, pager; + + ok(register_test_notify_class(), "Register test class failed, error 0x%08x\n", GetLastError()); + + parent = CreateWindowA(class, "parent", WS_OVERLAPPED, 0, 0, 100, 100, 0, 0, GetModuleHandleA(0), 0); + ok(parent != NULL, "CreateWindow failed\n"); + pager = CreateWindowA(WC_PAGESCROLLERA, "pager", WS_CHILD, 0, 0, 100, 100, parent, 0, GetModuleHandleA(0), 0); + ok(pager != NULL, "CreateWindow failed\n"); + child1_wnd = CreateWindowA(class, "child", WS_CHILD, 0, 0, 100, 100, pager, (HMENU)1, GetModuleHandleA(0), 0); + ok(child1_wnd != NULL, "CreateWindow failed\n"); + SendMessageW(pager, PGM_SETCHILD, 0, (LPARAM)child1_wnd); + + test_wm_notify_toolbar(pager); + + DestroyWindow(parent); + UnregisterClassA(class, GetModuleHandleA(NULL)); +} + static void init_functions(void) { HMODULE mod = LoadLibraryA("comctl32.dll"); @@ -447,6 +877,7 @@ START_TEST(pager) test_pager(); test_wm_notifyformat(); + test_wm_notify(); DestroyWindow(parent_wnd); } -- 2.18.0
On 8/28/2018 6:27 AM, Zhiyi Zhang wrote:
Fix BibleWorks 10 not displaying toolbar.
Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/comctl32/pager.c | 96 ++++++++ dlls/comctl32/tests/pager.c | 431 ++++++++++++++++++++++++++++++++++++ 2 files changed, 527 insertions(+)
diff --git a/dlls/comctl32/pager.c b/dlls/comctl32/pager.c index de63cad717..0367108db2 100644 --- a/dlls/comctl32/pager.c +++ b/dlls/comctl32/pager.c @@ -91,6 +91,17 @@ typedef struct #define INITIAL_DELAY 500 #define REPEAT_DELAY 50
+/* Text field conversion behavior flags for PAGER_SendConvertedNotify() */ +typedef enum +{ + /* Convert ANSI text from parent back to Unicode for pager */ + FROM_ANSI = 0x01, + /* Convert Unicode text to ANSI for parent before sending. If not set, do nothing */ + TO_ANSI = 0x02,
Isn't it always from or to ANSI? Maybe name it something like CONVERT_SEND CONVERT_RECEIVE?
+ /* Send empty text to parent if text is NULL, original text pointer still remain NULL */ + SET_NULL_EMPTY = 0x04 +} ConversionFlag;
This should be 'enum consersion_flags', no typedef.
+ static void PAGER_GetButtonRects(const PAGER_INFO* infoPtr, RECT* prcTopLeft, RECT* prcBottomRight, BOOL bClientCoords) { @@ -1020,6 +1031,89 @@ static LRESULT PAGER_NotifyFormat(PAGER_INFO *infoPtr, INT command) } }
+static UINT PAGER_GetAnsiNtfCode(UINT code) +{ + switch (code) + { + /* Toolbar */ + case TBN_GETBUTTONINFOW: return TBN_GETBUTTONINFOA; + case TBN_GETINFOTIPW: return TBN_GETINFOTIPA; + } + return code; +} + +static LRESULT PAGER_SendConvertedNotify(PAGER_INFO *infoPtr, NMHDR *hdr, WCHAR **text, INT *textMax, ConversionFlag flags)
It's better to have DWORD for flags.
+{ + WCHAR emptyW[] = {0};
Shouldn't it be constant?
+ WCHAR *oldText; + INT oldTextMax; + void *sendBuffer = NULL; + void *receiveBuffer = NULL;
Why are these untyped?
+ INT bufferSize; + LRESULT ret = 0; + + oldText = *text; + oldTextMax = textMax ? *textMax : 0; + + hdr->code = PAGER_GetAnsiNtfCode(hdr->code); + + if (oldTextMax < 0) goto done; + + if (!*text && flags & SET_NULL_EMPTY) + *text = emptyW; + + if (*text && flags & TO_ANSI) + { + bufferSize = WideCharToMultiByte(CP_ACP, 0, *text, -1, 0, 0, NULL, FALSE); + bufferSize = max(bufferSize, oldTextMax);
Why is this adjustment necessary?
+ sendBuffer = heap_alloc(bufferSize); + if (!sendBuffer) goto done; + WideCharToMultiByte(CP_ACP, 0, *text, -1, sendBuffer, bufferSize, NULL, FALSE); + *text = sendBuffer; + } + + ret = SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); + + if (*text && flags & FROM_ANSI) + { + bufferSize = lstrlenA((CHAR *)*text); + receiveBuffer = heap_alloc(bufferSize + 1); + if (!receiveBuffer) goto done; + memcpy(receiveBuffer, *text, bufferSize + 1); + if (!oldText) goto done; + MultiByteToWideChar(CP_ACP, 0, receiveBuffer, -1, oldText, oldTextMax);
Why do you need to make a copy here?
+ } + +done: + heap_free(sendBuffer); + heap_free(receiveBuffer); + *text = oldText; + return ret; +} + +static LRESULT PAGER_Notify(PAGER_INFO *infoPtr, NMHDR *hdr) +{ + if (infoPtr->bUnicode) return SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); + + switch (hdr->code) + { + /* Toolbar */ + case TBN_GETBUTTONINFOW: + { + NMTOOLBARW *nmtb = (NMTOOLBARW *)hdr; + return PAGER_SendConvertedNotify(infoPtr, hdr, &nmtb->pszText, &nmtb->cchText, + SET_NULL_EMPTY | TO_ANSI | FROM_ANSI); + } + case TBN_GETINFOTIPW: + { + NMTBGETINFOTIPW *nmtbgit = (NMTBGETINFOTIPW *)hdr; + return PAGER_SendConvertedNotify(infoPtr, hdr, &nmtbgit->pszText, &nmtbgit->cchTextMax, FROM_ANSI); + } + } + /* Other notifications or notifications above that don't have text, thus no need to convert */ + return SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); +} + static LRESULT WINAPI PAGER_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { @@ -1115,6 +1209,8 @@ PAGER_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) return PAGER_NotifyFormat (infoPtr, lParam);
case WM_NOTIFY: + return PAGER_Notify (infoPtr, (NMHDR *)lParam); + case WM_COMMAND: return SendMessageW (infoPtr->hwndNotify, uMsg, wParam, lParam);
diff --git a/dlls/comctl32/tests/pager.c b/dlls/comctl32/tests/pager.c index a1f6683a57..46f2eb0acb 100644 --- a/dlls/comctl32/tests/pager.c +++ b/dlls/comctl32/tests/pager.c @@ -30,10 +30,91 @@ static HWND parent_wnd, child1_wnd, child2_wnd; static INT notify_format; static BOOL notify_query_received; +static WCHAR test_w[] = {'t', 'e', 's', 't', 0}; +static CHAR test_a[] = {'t', 'e', 's', 't', 0}; +static WCHAR t_w[] = {'t', 0}; +static CHAR large_a[] = "You should have received a copy of the GNU Lesser General Public License along with this ..."; +static WCHAR buffer[64]; +static WCHAR buffer2[64]; + +/* Text field conversion behavior flags for PAGER_SendConvertedNotify() */ +typedef enum +{ + /* Convert ANSI text from parent back to Unicode for pager */ + FROM_ANSI = 0x01, + /* Convert Unicode text to ANSI for parent before sending. If not set, do nothing */ + TO_ANSI = 0x02, + /* Send empty text to parent if text is NULL, original text pointer still remain NULL */ + SET_NULL_EMPTY = 0x04 +} ConversionFlag; + +static struct notify_test_data +{ + UINT unicode; + UINT ansi; + UINT_PTR id_from; + HWND hwnd_from; + INT text_size; + /* Whether parent receive notification */ + BOOL received; + UINT sub_test_id; + /* Text field conversion behavior flags */ + ConversionFlag flags; +} notify_test_data;
#define CHILD1_ID 1 #define CHILD2_ID 2
+#define expect_eq_int(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " 0x%08x, got 0x%08x\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_long(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " 0x%08lx, got 0x%08lx\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_pointer(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " %p, got %p\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_text_a(actual, expected) \ + do \ + { \ + ok(!lstrcmpA((actual), (expected)), "Code:0x%08x Subtest:0x%08x Expect " #actual " %s, got %s\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_text_w(actual, expected) \ + do \ + { \ + ok(!lstrcmpW((actual), (expected)), "Code:0x%08x Subtest:0x%08x Expect " #actual " %s, got %s\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, wine_dbgstr_w((expected)), \ + wine_dbgstr_w((actual))); \ + } while (0) + +#define expect_text_empty(actual) \ + do \ + { \ + ok((actual) && !(actual)[0], "Code:0x%08x Subtest:0x%08x Expect " #actual " empty\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id); \ + } while (0) + +#define expect_null(actual) \ + do \ + { \ + ok(!(actual), "Code:0x%08x Subtest:0x%08x Expect " #actual " NULL, got %p\n", notify_test_data.unicode, \ + notify_test_data.sub_test_id, (actual)); \ + } while (0) +
Do we really need this? I'd rather had them inlined.
static BOOL (WINAPI *pInitCommonControlsEx)(const INITCOMMONCONTROLSEX*); static BOOL (WINAPI *pSetWindowSubclass)(HWND, SUBCLASSPROC, UINT_PTR, DWORD_PTR);
@@ -419,6 +500,355 @@ static void test_wm_notifyformat(void) UnregisterClassW(class_w, GetModuleHandleW(NULL)); }
+static void wm_notify_text_handler(CHAR **text, INT *textMax) +{ + expect_eq_int(*textMax, notify_test_data.text_size); + switch (notify_test_data.sub_test_id) + { + case 1: + if (notify_test_data.flags & SET_NULL_EMPTY) + expect_text_empty(*text); + else + expect_null(*text); + break; + case 2: + { + INT length = lstrlenA(*text); + ok(length == 0 + || broken(length == 3) /* 2003 */ + || broken(length == 1) /* <= Win7 */, + "Wrong text length\n"); + break; + } + case 3: + if (notify_test_data.flags & TO_ANSI) + { + expect_eq_text_a(*text, test_a); + ok(*text != (CHAR *)buffer, "Expect using a new buffer\n"); + } + else + { + expect_eq_text_w((WCHAR *)*text, test_w); + expect_eq_pointer(*text, (CHAR *)buffer); + } + break; + case 4: + expect_text_empty(*text); + memcpy(*text, test_a, sizeof(test_a)); + break; + case 5: + expect_text_empty(*text); + *textMax = 1; + break; + case 6: + expect_text_empty(*text); + *text = test_a; + break; + case 7: + *text = test_a; + break; + case 8: + expect_text_empty(*text); + *text = large_a; + break; + } +}
I don't like those test id's spread like that. Can we have static test data that we'll iterate through, and symbolic names for test ids?
+ +static LRESULT WINAPI test_notify_proc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) +{ + static const WCHAR test[] = {'t', 'e', 's', 't', 0}; + switch (message) + { + case WM_NOTIFY: + { + NMHDR *hdr = (NMHDR *)lParam; + + /* Not notifications we want to test */ + if (!notify_test_data.unicode) break; + ok(!notify_test_data.received, "Extra notification received\n"); + + expect_eq_long(wParam, notify_test_data.id_from); + expect_eq_int(hdr->code, notify_test_data.ansi); + expect_eq_long(hdr->idFrom, notify_test_data.id_from); + expect_eq_pointer(hdr->hwndFrom, notify_test_data.hwnd_from); + + if (hdr->code != notify_test_data.ansi) + { + skip("Notification code mismatch, skipping lParam check\n"); + return 0; + } + switch (hdr->code) + { + /* Toolbar */ + /* No conversion for TBN_SAVE and TBN_RESTORE */ + case TBN_SAVE: + { + NMTBSAVE *nmtbs = (NMTBSAVE *)hdr; + switch (notify_test_data.sub_test_id) + { + case 0: + expect_eq_text_w((WCHAR *)nmtbs->tbButton.iString, test_w); + break; + case 1: + nmtbs->tbButton.iString = (INT_PTR)test_a; + break; + } + break; + } + case TBN_RESTORE: + { + NMTBRESTORE *nmtbr = (NMTBRESTORE *)hdr; + switch (notify_test_data.sub_test_id) + { + case 0: + expect_eq_text_w((WCHAR *)nmtbr->tbButton.iString, test_w); + break; + case 1: + nmtbr->tbButton.iString = (INT_PTR)test_a; + break; + } + break; + } + case TBN_GETBUTTONINFOA: + { + NMTOOLBARA *nmtb = (NMTOOLBARA *)hdr; + wm_notify_text_handler(&nmtb->pszText, &nmtb->cchText); + break; + } + /* TBN_GETDISPINFOW doesn't get converted to TBN_GETDISPINFOA */ + case TBN_GETDISPINFOW: + { + NMTBDISPINFOW *nmtbdi = (NMTBDISPINFOW *)hdr; + expect_eq_text_w(nmtbdi->pszText, test_w); + expect_eq_int(nmtbdi->cchText, ARRAY_SIZE(buffer)); + memcpy(nmtbdi->pszText, test_a, sizeof(test_a)); + break; + } + case TBN_GETINFOTIPA: + { + NMTBGETINFOTIPA *nmtbgit = (NMTBGETINFOTIPA *)hdr; + wm_notify_text_handler(&nmtbgit->pszText, &nmtbgit->cchTextMax); + break; + } + default: + ok(0, "Unexpected message 0x%08x\n", hdr->code); + } + notify_test_data.received = TRUE; + ok(!lstrcmpA(test_a, "test"), "test_a got modified\n"); + ok(!lstrcmpW(test_w, test), "test_w got modified\n"); + return 0; + } + case WM_NOTIFYFORMAT: + if (lParam == NF_QUERY) return NFR_ANSI; + break; + } + return DefWindowProcA(hwnd, message, wParam, lParam); +} + +static BOOL register_test_notify_class(void) +{ + WNDCLASSA cls = {0}; + + cls.lpfnWndProc = test_notify_proc; + cls.hInstance = GetModuleHandleA(NULL); + cls.lpszClassName = "Pager notify class"; + return RegisterClassA(&cls); +} + +static void send_notify_(HWND pager, UINT unicode, UINT ansi, LPARAM lParam, BOOL code_change, BOOL received) +{ + NMHDR *hdr = (NMHDR *)lParam; + + notify_test_data.unicode = unicode; + notify_test_data.id_from = 1; + notify_test_data.hwnd_from = child1_wnd; + notify_test_data.ansi = ansi; + notify_test_data.received = FALSE; + + hdr->code = unicode; + hdr->idFrom = 1; + hdr->hwndFrom = child1_wnd; + + SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam); + expect_eq_int(notify_test_data.received, received); + expect_eq_int(hdr->code, code_change ? notify_test_data.ansi : notify_test_data.unicode); +} + +#define send_notify(a, b, c, d, e) send_notify_(pager, a, b, c, d, e) + +/* Send notify to test text field conversion. In parent proc wm_notify_text_handler() handles these messages */ +static void test_wm_notify_text_helper_(HWND pager, void *ptr, size_t size, WCHAR **text, INT *textMax, + UINT code_unicode, UINT code_ansi, ConversionFlag flags) +{ + notify_test_data.flags = flags; + + /* NULL text */ + notify_test_data.sub_test_id = 1; + memset(ptr, 0, size); + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_null(*text); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Zero text max size */ + notify_test_data.sub_test_id = 2; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + /* Got *text null on Windows <= Win7 */ + ok(broken(!*text) || (!lstrlenW(*text) && *text == buffer), "Wrong *text\n"); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification */ + notify_test_data.sub_test_id = 3; + memset(ptr, 0, size); + memcpy(buffer, test_w, sizeof(test_w)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + if (!(flags & TO_ANSI) && flags & FROM_ANSI) + expect_eq_text_w(*text, t_w); + else + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent write to buffer */ + notify_test_data.sub_test_id = 4; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent set buffer size to one */ + notify_test_data.sub_test_id = 5; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, 1); + + /* Valid notification. Parent replace buffer */ + notify_test_data.sub_test_id = 6; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* NULL text. Parent replace buffer */ + notify_test_data.sub_test_id = 7; + memset(ptr, 0, size); + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_null(*text); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent replace buffer with a pointer pointing to a string larger than old buffer can store */ + notify_test_data.sub_test_id = 8; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + memset(buffer2, 0, sizeof(buffer2)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + MultiByteToWideChar(CP_ACP, 0, large_a, -1, buffer2, ARRAY_SIZE(buffer2)); + ok(!memcmp(*text, buffer2, sizeof(buffer2)), "Expect memory content to be the same\n"); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); +} + +#define test_wm_notify_text_helper(a, b, c, d, e, f, g) test_wm_notify_text_helper_(pager, a, b, c, d, e, f, g) + +static void test_wm_notify_toolbar(HWND pager) +{ + NMTBRESTORE nmtbr; + NMTBSAVE nmtbs; + NMTOOLBARW nmtb; + NMTBDISPINFOW nmtbdi; + NMTBGETINFOTIPW nmtbgit; + + /* No conversion for TBN_SAVE */ + notify_test_data.sub_test_id = 0; + memset(&nmtbs, 0, sizeof(nmtbs)); + nmtbs.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_SAVE, TBN_SAVE, (LPARAM)&nmtbs, TRUE, TRUE); + expect_eq_text_w((WCHAR *)nmtbs.tbButton.iString, test_w); + + notify_test_data.sub_test_id = 1; + memset(&nmtbs, 0, sizeof(nmtbs)); + nmtbs.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_SAVE, TBN_SAVE, (LPARAM)&nmtbs, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbs.tbButton.iString, test_a); + + /* No conversion for TBN_RESTORE */ + notify_test_data.sub_test_id = 0; + memset(&nmtbr, 0, sizeof(nmtbr)); + nmtbr.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_RESTORE, TBN_RESTORE, (LPARAM)&nmtbr, TRUE, TRUE); + expect_eq_text_w((WCHAR *)nmtbr.tbButton.iString, test_w); + + notify_test_data.sub_test_id = 1; + memset(&nmtbr, 0, sizeof(nmtbr)); + nmtbr.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_RESTORE, TBN_RESTORE, (LPARAM)&nmtbr, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbr.tbButton.iString, test_a); + + /* TBN_GETDISPINFOW doesn't get unconverted */ + memset(&nmtbdi, 0, sizeof(nmtbdi)); + memcpy(buffer, test_w, sizeof(test_w)); + nmtbdi.dwMask = TBNF_TEXT; + nmtbdi.pszText = buffer; + nmtbdi.cchText = ARRAY_SIZE(buffer); + notify_test_data.text_size = nmtbdi.cchText; + send_notify(TBN_GETDISPINFOW, TBN_GETDISPINFOW, (LPARAM)&nmtbdi, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbdi.pszText, test_a); + + test_wm_notify_text_helper(&nmtb, sizeof(nmtb), &nmtb.pszText, &nmtb.cchText, TBN_GETBUTTONINFOW, + TBN_GETBUTTONINFOA, SET_NULL_EMPTY | TO_ANSI); + test_wm_notify_text_helper(&nmtbgit, sizeof(nmtbgit), &nmtbgit.pszText, &nmtbgit.cchTextMax, TBN_GETINFOTIPW, + TBN_GETINFOTIPA, FROM_ANSI); +} + +static void test_wm_notify(void) +{ + static const CHAR *class = "Pager notify class"; + HWND parent, pager; + + ok(register_test_notify_class(), "Register test class failed, error 0x%08x\n", GetLastError()); + + parent = CreateWindowA(class, "parent", WS_OVERLAPPED, 0, 0, 100, 100, 0, 0, GetModuleHandleA(0), 0); + ok(parent != NULL, "CreateWindow failed\n"); + pager = CreateWindowA(WC_PAGESCROLLERA, "pager", WS_CHILD, 0, 0, 100, 100, parent, 0, GetModuleHandleA(0), 0); + ok(pager != NULL, "CreateWindow failed\n"); + child1_wnd = CreateWindowA(class, "child", WS_CHILD, 0, 0, 100, 100, pager, (HMENU)1, GetModuleHandleA(0), 0); + ok(child1_wnd != NULL, "CreateWindow failed\n"); + SendMessageW(pager, PGM_SETCHILD, 0, (LPARAM)child1_wnd); + + test_wm_notify_toolbar(pager); + + DestroyWindow(parent); + UnregisterClassA(class, GetModuleHandleA(NULL)); +} + static void init_functions(void) { HMODULE mod = LoadLibraryA("comctl32.dll"); @@ -447,6 +877,7 @@ START_TEST(pager)
test_pager(); test_wm_notifyformat(); + test_wm_notify();
DestroyWindow(parent_wnd); }
On Tue 8 28 14:41, Nikolay Sivov wrote:
On 8/28/2018 6:27 AM, Zhiyi Zhang wrote:
Fix BibleWorks 10 not displaying toolbar.
Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/comctl32/pager.c | 96 ++++++++ dlls/comctl32/tests/pager.c | 431 ++++++++++++++++++++++++++++++++++++ 2 files changed, 527 insertions(+)
diff --git a/dlls/comctl32/pager.c b/dlls/comctl32/pager.c index de63cad717..0367108db2 100644 --- a/dlls/comctl32/pager.c +++ b/dlls/comctl32/pager.c @@ -91,6 +91,17 @@ typedef struct #define INITIAL_DELAY 500 #define REPEAT_DELAY 50
+/* Text field conversion behavior flags for PAGER_SendConvertedNotify() */ +typedef enum +{ + /* Convert ANSI text from parent back to Unicode for pager */ + FROM_ANSI = 0x01, + /* Convert Unicode text to ANSI for parent before sending. If not set, do nothing */ + TO_ANSI = 0x02,
Isn't it always from or to ANSI? Maybe name it something like CONVERT_SEND CONVERT_RECEIVE?
Yes, OK.
+ /* Send empty text to parent if text is NULL, original text pointer still remain NULL */ + SET_NULL_EMPTY = 0x04 +} ConversionFlag;
This should be 'enum consersion_flags', no typedef.
OK.
+ static void PAGER_GetButtonRects(const PAGER_INFO* infoPtr, RECT* prcTopLeft, RECT* prcBottomRight, BOOL bClientCoords) { @@ -1020,6 +1031,89 @@ static LRESULT PAGER_NotifyFormat(PAGER_INFO *infoPtr, INT command) } }
+static UINT PAGER_GetAnsiNtfCode(UINT code) +{ + switch (code) + { + /* Toolbar */ + case TBN_GETBUTTONINFOW: return TBN_GETBUTTONINFOA; + case TBN_GETINFOTIPW: return TBN_GETINFOTIPA; + } + return code; +} + +static LRESULT PAGER_SendConvertedNotify(PAGER_INFO *infoPtr, NMHDR *hdr, WCHAR **text, INT *textMax, ConversionFlag flags)
It's better to have DWORD for flags.
Sure.
+{ + WCHAR emptyW[] = {0};
Shouldn't it be constant?
emptyW would be assign to the text pointer passed to application. text pointer is not const, thus a compiler warning.
+ WCHAR *oldText; + INT oldTextMax; + void *sendBuffer = NULL; + void *receiveBuffer = NULL;
Why are these untyped? To avoid typecasting. They can be typed, sure.
+ INT bufferSize; + LRESULT ret = 0; + + oldText = *text; + oldTextMax = textMax ? *textMax : 0; + + hdr->code = PAGER_GetAnsiNtfCode(hdr->code); + + if (oldTextMax < 0) goto done; + + if (!*text && flags & SET_NULL_EMPTY) + *text = emptyW; + + if (*text && flags & TO_ANSI) + { + bufferSize = WideCharToMultiByte(CP_ACP, 0, *text, -1, 0, 0, NULL, FALSE); + bufferSize = max(bufferSize, oldTextMax);
Why is this adjustment necessary?
Application may write oldTextMax characters, since oldTextMax is passed to application unchanged. And when widechar string converted to multibyte, there might be more than oldTextMax bytes.
+ sendBuffer = heap_alloc(bufferSize); + if (!sendBuffer) goto done; + WideCharToMultiByte(CP_ACP, 0, *text, -1, sendBuffer, bufferSize, NULL, FALSE); + *text = sendBuffer; + } + + ret = SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); + + if (*text && flags & FROM_ANSI) + { + bufferSize = lstrlenA((CHAR *)*text); + receiveBuffer = heap_alloc(bufferSize + 1); + if (!receiveBuffer) goto done; + memcpy(receiveBuffer, *text, bufferSize + 1); + if (!oldText) goto done; + MultiByteToWideChar(CP_ACP, 0, receiveBuffer, -1, oldText, oldTextMax);
Why do you need to make a copy here?
*text is application provided buffer pointer, which may or may not be a different buffer pointer because application can replace the original buffer pointer. And MultiByteToWideChar can't use the same buffer for source and destination. But I think if buffer pointer is different, we can avoid a copy.
+ } + +done: + heap_free(sendBuffer); + heap_free(receiveBuffer); + *text = oldText; + return ret; +} + +static LRESULT PAGER_Notify(PAGER_INFO *infoPtr, NMHDR *hdr) +{ + if (infoPtr->bUnicode) return SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); + + switch (hdr->code) + { + /* Toolbar */ + case TBN_GETBUTTONINFOW: + { + NMTOOLBARW *nmtb = (NMTOOLBARW *)hdr; + return PAGER_SendConvertedNotify(infoPtr, hdr, &nmtb->pszText, &nmtb->cchText, + SET_NULL_EMPTY | TO_ANSI | FROM_ANSI); + } + case TBN_GETINFOTIPW: + { + NMTBGETINFOTIPW *nmtbgit = (NMTBGETINFOTIPW *)hdr; + return PAGER_SendConvertedNotify(infoPtr, hdr, &nmtbgit->pszText, &nmtbgit->cchTextMax, FROM_ANSI); + } + } + /* Other notifications or notifications above that don't have text, thus no need to convert */ + return SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr); +} + static LRESULT WINAPI PAGER_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) { @@ -1115,6 +1209,8 @@ PAGER_WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam) return PAGER_NotifyFormat (infoPtr, lParam);
case WM_NOTIFY: + return PAGER_Notify (infoPtr, (NMHDR *)lParam); + case WM_COMMAND: return SendMessageW (infoPtr->hwndNotify, uMsg, wParam, lParam);
diff --git a/dlls/comctl32/tests/pager.c b/dlls/comctl32/tests/pager.c index a1f6683a57..46f2eb0acb 100644 --- a/dlls/comctl32/tests/pager.c +++ b/dlls/comctl32/tests/pager.c @@ -30,10 +30,91 @@ static HWND parent_wnd, child1_wnd, child2_wnd; static INT notify_format; static BOOL notify_query_received; +static WCHAR test_w[] = {'t', 'e', 's', 't', 0}; +static CHAR test_a[] = {'t', 'e', 's', 't', 0}; +static WCHAR t_w[] = {'t', 0}; +static CHAR large_a[] = "You should have received a copy of the GNU Lesser General Public License along with this ..."; +static WCHAR buffer[64]; +static WCHAR buffer2[64]; + +/* Text field conversion behavior flags for PAGER_SendConvertedNotify() */ +typedef enum +{ + /* Convert ANSI text from parent back to Unicode for pager */ + FROM_ANSI = 0x01, + /* Convert Unicode text to ANSI for parent before sending. If not set, do nothing */ + TO_ANSI = 0x02, + /* Send empty text to parent if text is NULL, original text pointer still remain NULL */ + SET_NULL_EMPTY = 0x04 +} ConversionFlag; + +static struct notify_test_data +{ + UINT unicode; + UINT ansi; + UINT_PTR id_from; + HWND hwnd_from; + INT text_size; + /* Whether parent receive notification */ + BOOL received; + UINT sub_test_id; + /* Text field conversion behavior flags */ + ConversionFlag flags; +} notify_test_data;
#define CHILD1_ID 1 #define CHILD2_ID 2
+#define expect_eq_int(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " 0x%08x, got 0x%08x\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_long(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " 0x%08lx, got 0x%08lx\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_pointer(actual, expected) \ + do \ + { \ + ok((actual) == (expected), "Code:0x%08x Subtest:0x%08x Expect " #actual " %p, got %p\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_text_a(actual, expected) \ + do \ + { \ + ok(!lstrcmpA((actual), (expected)), "Code:0x%08x Subtest:0x%08x Expect " #actual " %s, got %s\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, (expected), (actual)); \ + } while (0) + +#define expect_eq_text_w(actual, expected) \ + do \ + { \ + ok(!lstrcmpW((actual), (expected)), "Code:0x%08x Subtest:0x%08x Expect " #actual " %s, got %s\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id, wine_dbgstr_w((expected)), \ + wine_dbgstr_w((actual))); \ + } while (0) + +#define expect_text_empty(actual) \ + do \ + { \ + ok((actual) && !(actual)[0], "Code:0x%08x Subtest:0x%08x Expect " #actual " empty\n", \ + notify_test_data.unicode, notify_test_data.sub_test_id); \ + } while (0) + +#define expect_null(actual) \ + do \ + { \ + ok(!(actual), "Code:0x%08x Subtest:0x%08x Expect " #actual " NULL, got %p\n", notify_test_data.unicode, \ + notify_test_data.sub_test_id, (actual)); \ + } while (0) +
Do we really need this? I'd rather had them inlined.
They're used extensively. Having them inline seems to be more code.
static BOOL (WINAPI *pInitCommonControlsEx)(const INITCOMMONCONTROLSEX*); static BOOL (WINAPI *pSetWindowSubclass)(HWND, SUBCLASSPROC, UINT_PTR, DWORD_PTR);
@@ -419,6 +500,355 @@ static void test_wm_notifyformat(void) UnregisterClassW(class_w, GetModuleHandleW(NULL)); }
+static void wm_notify_text_handler(CHAR **text, INT *textMax) +{ + expect_eq_int(*textMax, notify_test_data.text_size); + switch (notify_test_data.sub_test_id) + { + case 1: + if (notify_test_data.flags & SET_NULL_EMPTY) + expect_text_empty(*text); + else + expect_null(*text); + break; + case 2: + { + INT length = lstrlenA(*text); + ok(length == 0 + || broken(length == 3) /* 2003 */ + || broken(length == 1) /* <= Win7 */, + "Wrong text length\n"); + break; + } + case 3: + if (notify_test_data.flags & TO_ANSI) + { + expect_eq_text_a(*text, test_a); + ok(*text != (CHAR *)buffer, "Expect using a new buffer\n"); + } + else + { + expect_eq_text_w((WCHAR *)*text, test_w); + expect_eq_pointer(*text, (CHAR *)buffer); + } + break; + case 4: + expect_text_empty(*text); + memcpy(*text, test_a, sizeof(test_a)); + break; + case 5: + expect_text_empty(*text); + *textMax = 1; + break; + case 6: + expect_text_empty(*text); + *text = test_a; + break; + case 7: + *text = test_a; + break; + case 8: + expect_text_empty(*text); + *text = large_a; + break; + } +}
I don't like those test id's spread like that. Can we have static test data that we'll iterate through, and symbolic names for test ids?
Ehh, they have rather different behaviors. Static test data doesn't seem so great either. I'll think of something to simplify the tests.
+ +static LRESULT WINAPI test_notify_proc(HWND hwnd, UINT message, WPARAM wParam, LPARAM lParam) +{ + static const WCHAR test[] = {'t', 'e', 's', 't', 0}; + switch (message) + { + case WM_NOTIFY: + { + NMHDR *hdr = (NMHDR *)lParam; + + /* Not notifications we want to test */ + if (!notify_test_data.unicode) break; + ok(!notify_test_data.received, "Extra notification received\n"); + + expect_eq_long(wParam, notify_test_data.id_from); + expect_eq_int(hdr->code, notify_test_data.ansi); + expect_eq_long(hdr->idFrom, notify_test_data.id_from); + expect_eq_pointer(hdr->hwndFrom, notify_test_data.hwnd_from); + + if (hdr->code != notify_test_data.ansi) + { + skip("Notification code mismatch, skipping lParam check\n"); + return 0; + } + switch (hdr->code) + { + /* Toolbar */ + /* No conversion for TBN_SAVE and TBN_RESTORE */ + case TBN_SAVE: + { + NMTBSAVE *nmtbs = (NMTBSAVE *)hdr; + switch (notify_test_data.sub_test_id) + { + case 0: + expect_eq_text_w((WCHAR *)nmtbs->tbButton.iString, test_w); + break; + case 1: + nmtbs->tbButton.iString = (INT_PTR)test_a; + break; + } + break; + } + case TBN_RESTORE: + { + NMTBRESTORE *nmtbr = (NMTBRESTORE *)hdr; + switch (notify_test_data.sub_test_id) + { + case 0: + expect_eq_text_w((WCHAR *)nmtbr->tbButton.iString, test_w); + break; + case 1: + nmtbr->tbButton.iString = (INT_PTR)test_a; + break; + } + break; + } + case TBN_GETBUTTONINFOA: + { + NMTOOLBARA *nmtb = (NMTOOLBARA *)hdr; + wm_notify_text_handler(&nmtb->pszText, &nmtb->cchText); + break; + } + /* TBN_GETDISPINFOW doesn't get converted to TBN_GETDISPINFOA */ + case TBN_GETDISPINFOW: + { + NMTBDISPINFOW *nmtbdi = (NMTBDISPINFOW *)hdr; + expect_eq_text_w(nmtbdi->pszText, test_w); + expect_eq_int(nmtbdi->cchText, ARRAY_SIZE(buffer)); + memcpy(nmtbdi->pszText, test_a, sizeof(test_a)); + break; + } + case TBN_GETINFOTIPA: + { + NMTBGETINFOTIPA *nmtbgit = (NMTBGETINFOTIPA *)hdr; + wm_notify_text_handler(&nmtbgit->pszText, &nmtbgit->cchTextMax); + break; + } + default: + ok(0, "Unexpected message 0x%08x\n", hdr->code); + } + notify_test_data.received = TRUE; + ok(!lstrcmpA(test_a, "test"), "test_a got modified\n"); + ok(!lstrcmpW(test_w, test), "test_w got modified\n"); + return 0; + } + case WM_NOTIFYFORMAT: + if (lParam == NF_QUERY) return NFR_ANSI; + break; + } + return DefWindowProcA(hwnd, message, wParam, lParam); +} + +static BOOL register_test_notify_class(void) +{ + WNDCLASSA cls = {0}; + + cls.lpfnWndProc = test_notify_proc; + cls.hInstance = GetModuleHandleA(NULL); + cls.lpszClassName = "Pager notify class"; + return RegisterClassA(&cls); +} + +static void send_notify_(HWND pager, UINT unicode, UINT ansi, LPARAM lParam, BOOL code_change, BOOL received) +{ + NMHDR *hdr = (NMHDR *)lParam; + + notify_test_data.unicode = unicode; + notify_test_data.id_from = 1; + notify_test_data.hwnd_from = child1_wnd; + notify_test_data.ansi = ansi; + notify_test_data.received = FALSE; + + hdr->code = unicode; + hdr->idFrom = 1; + hdr->hwndFrom = child1_wnd; + + SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam); + expect_eq_int(notify_test_data.received, received); + expect_eq_int(hdr->code, code_change ? notify_test_data.ansi : notify_test_data.unicode); +} + +#define send_notify(a, b, c, d, e) send_notify_(pager, a, b, c, d, e) + +/* Send notify to test text field conversion. In parent proc wm_notify_text_handler() handles these messages */ +static void test_wm_notify_text_helper_(HWND pager, void *ptr, size_t size, WCHAR **text, INT *textMax, + UINT code_unicode, UINT code_ansi, ConversionFlag flags) +{ + notify_test_data.flags = flags; + + /* NULL text */ + notify_test_data.sub_test_id = 1; + memset(ptr, 0, size); + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_null(*text); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Zero text max size */ + notify_test_data.sub_test_id = 2; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + /* Got *text null on Windows <= Win7 */ + ok(broken(!*text) || (!lstrlenW(*text) && *text == buffer), "Wrong *text\n"); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification */ + notify_test_data.sub_test_id = 3; + memset(ptr, 0, size); + memcpy(buffer, test_w, sizeof(test_w)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + if (!(flags & TO_ANSI) && flags & FROM_ANSI) + expect_eq_text_w(*text, t_w); + else + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent write to buffer */ + notify_test_data.sub_test_id = 4; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent set buffer size to one */ + notify_test_data.sub_test_id = 5; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, 1); + + /* Valid notification. Parent replace buffer */ + notify_test_data.sub_test_id = 6; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_eq_text_w(*text, test_w); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* NULL text. Parent replace buffer */ + notify_test_data.sub_test_id = 7; + memset(ptr, 0, size); + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + expect_null(*text); + expect_eq_int(*textMax, notify_test_data.text_size); + + /* Valid notification. Parent replace buffer with a pointer pointing to a string larger than old buffer can store */ + notify_test_data.sub_test_id = 8; + memset(ptr, 0, size); + memset(buffer, 0, sizeof(buffer)); + memset(buffer2, 0, sizeof(buffer2)); + *text = buffer; + *textMax = ARRAY_SIZE(buffer); + notify_test_data.text_size = *textMax; + send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE, TRUE); + MultiByteToWideChar(CP_ACP, 0, large_a, -1, buffer2, ARRAY_SIZE(buffer2)); + ok(!memcmp(*text, buffer2, sizeof(buffer2)), "Expect memory content to be the same\n"); + expect_eq_pointer(*text, buffer); + expect_eq_int(*textMax, notify_test_data.text_size); +} + +#define test_wm_notify_text_helper(a, b, c, d, e, f, g) test_wm_notify_text_helper_(pager, a, b, c, d, e, f, g) + +static void test_wm_notify_toolbar(HWND pager) +{ + NMTBRESTORE nmtbr; + NMTBSAVE nmtbs; + NMTOOLBARW nmtb; + NMTBDISPINFOW nmtbdi; + NMTBGETINFOTIPW nmtbgit; + + /* No conversion for TBN_SAVE */ + notify_test_data.sub_test_id = 0; + memset(&nmtbs, 0, sizeof(nmtbs)); + nmtbs.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_SAVE, TBN_SAVE, (LPARAM)&nmtbs, TRUE, TRUE); + expect_eq_text_w((WCHAR *)nmtbs.tbButton.iString, test_w); + + notify_test_data.sub_test_id = 1; + memset(&nmtbs, 0, sizeof(nmtbs)); + nmtbs.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_SAVE, TBN_SAVE, (LPARAM)&nmtbs, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbs.tbButton.iString, test_a); + + /* No conversion for TBN_RESTORE */ + notify_test_data.sub_test_id = 0; + memset(&nmtbr, 0, sizeof(nmtbr)); + nmtbr.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_RESTORE, TBN_RESTORE, (LPARAM)&nmtbr, TRUE, TRUE); + expect_eq_text_w((WCHAR *)nmtbr.tbButton.iString, test_w); + + notify_test_data.sub_test_id = 1; + memset(&nmtbr, 0, sizeof(nmtbr)); + nmtbr.tbButton.iString = (INT_PTR)test_w; + send_notify(TBN_RESTORE, TBN_RESTORE, (LPARAM)&nmtbr, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbr.tbButton.iString, test_a); + + /* TBN_GETDISPINFOW doesn't get unconverted */ + memset(&nmtbdi, 0, sizeof(nmtbdi)); + memcpy(buffer, test_w, sizeof(test_w)); + nmtbdi.dwMask = TBNF_TEXT; + nmtbdi.pszText = buffer; + nmtbdi.cchText = ARRAY_SIZE(buffer); + notify_test_data.text_size = nmtbdi.cchText; + send_notify(TBN_GETDISPINFOW, TBN_GETDISPINFOW, (LPARAM)&nmtbdi, TRUE, TRUE); + expect_eq_text_a((CHAR *)nmtbdi.pszText, test_a); + + test_wm_notify_text_helper(&nmtb, sizeof(nmtb), &nmtb.pszText, &nmtb.cchText, TBN_GETBUTTONINFOW, + TBN_GETBUTTONINFOA, SET_NULL_EMPTY | TO_ANSI); + test_wm_notify_text_helper(&nmtbgit, sizeof(nmtbgit), &nmtbgit.pszText, &nmtbgit.cchTextMax, TBN_GETINFOTIPW, + TBN_GETINFOTIPA, FROM_ANSI); +} + +static void test_wm_notify(void) +{ + static const CHAR *class = "Pager notify class"; + HWND parent, pager; + + ok(register_test_notify_class(), "Register test class failed, error 0x%08x\n", GetLastError()); + + parent = CreateWindowA(class, "parent", WS_OVERLAPPED, 0, 0, 100, 100, 0, 0, GetModuleHandleA(0), 0); + ok(parent != NULL, "CreateWindow failed\n"); + pager = CreateWindowA(WC_PAGESCROLLERA, "pager", WS_CHILD, 0, 0, 100, 100, parent, 0, GetModuleHandleA(0), 0); + ok(pager != NULL, "CreateWindow failed\n"); + child1_wnd = CreateWindowA(class, "child", WS_CHILD, 0, 0, 100, 100, pager, (HMENU)1, GetModuleHandleA(0), 0); + ok(child1_wnd != NULL, "CreateWindow failed\n"); + SendMessageW(pager, PGM_SETCHILD, 0, (LPARAM)child1_wnd); + + test_wm_notify_toolbar(pager); + + DestroyWindow(parent); + UnregisterClassA(class, GetModuleHandleA(NULL)); +} + static void init_functions(void) { HMODULE mod = LoadLibraryA("comctl32.dll"); @@ -447,6 +877,7 @@ START_TEST(pager)
test_pager(); test_wm_notifyformat(); + test_wm_notify();
DestroyWindow(parent_wnd); }
participants (2)
-
Nikolay Sivov -
Zhiyi Zhang