[PATCH v2 1/3] comctl32/pager: Support toolbar notification conversion.

[Zhiyi Zhang]

Fix BibleWorks 10 not displaying toolbar.

Pager always wants Unicode notifications from its children.
And the parent of pager may want ANSI notifications. In this
case, pager need to convert Unicode notifications to ANSI ones
before sending them to parent and convert ANSI notifications
from parent to Unicode before returning the result to its children.

Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com>
---
v2: Rewrote tests with static test data.

 dlls/comctl32/pager.c       |  96 +++++++++
 dlls/comctl32/tests/pager.c | 404 ++++++++++++++++++++++++++++++++++++
 2 files changed, 500 insertions(+)

diff --git a/dlls/comctl32/pager.c b/dlls/comctl32/pager.c
index de63cad717..be9bccbbc7 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() */
+enum conversion_flags
+{
+    /* Convert Unicode text to ANSI for parent before sending. If not set, do nothing */
+    CONVERT_SEND = 0x01,
+    /* Convert ANSI text from parent back to Unicode for children */
+    CONVERT_RECEIVE = 0x02,
+    /* Send empty text to parent if text is NULL. Original text pointer still remains NULL */
+    NULL_SEND_EMPTY = 0x04
+};
+
 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, DWORD flags)
+{
+    CHAR *sendBuffer = NULL;
+    CHAR *receiveBuffer;
+    INT bufferSize;
+    WCHAR *oldText;
+    INT oldTextMax;
+    LRESULT ret = NO_ERROR;
+
+    oldText = *text;
+    oldTextMax = textMax ? *textMax : 0;
+
+    hdr->code = PAGER_GetAnsiNtfCode(hdr->code);
+
+    if (oldTextMax < 0) goto done;
+
+    if ((*text && flags & CONVERT_SEND) || (!*text && flags & NULL_SEND_EMPTY))
+    {
+        bufferSize = textMax ? *textMax : lstrlenW(*text) + 1;
+        sendBuffer = heap_alloc_zero(bufferSize);
+        if (!sendBuffer) goto done;
+        WideCharToMultiByte(CP_ACP, 0, *text, -1, sendBuffer, bufferSize, NULL, FALSE);
+        *text = (WCHAR *)sendBuffer;
+    }
+
+    ret = SendMessageW(infoPtr->hwndNotify, WM_NOTIFY, hdr->idFrom, (LPARAM)hdr);
+
+    if (*text && oldText && flags & CONVERT_RECEIVE)
+    {
+        /* MultiByteToWideChar requires that source and destination are not the same buffer */
+        if (*text == oldText)
+        {
+            bufferSize = lstrlenA((CHAR *)*text)  + 1;
+            receiveBuffer = heap_alloc_zero(bufferSize);
+            if (!receiveBuffer) goto done;
+            memcpy(receiveBuffer, *text, bufferSize);
+            MultiByteToWideChar(CP_ACP, 0, receiveBuffer, bufferSize, oldText, oldTextMax);
+            heap_free(receiveBuffer);
+        }
+        else
+            MultiByteToWideChar(CP_ACP, 0, (CHAR *)*text, -1, oldText, oldTextMax);
+    }
+
+done:
+    heap_free(sendBuffer);
+    *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,
+                                         NULL_SEND_EMPTY | CONVERT_SEND | CONVERT_RECEIVE);
+    }
+    case TBN_GETINFOTIPW:
+    {
+        NMTBGETINFOTIPW *nmtbgit = (NMTBGETINFOTIPW *)hdr;
+        return PAGER_SendConvertedNotify(infoPtr, hdr, &nmtbgit->pszText, &nmtbgit->cchTextMax, CONVERT_RECEIVE);
+    }
+    }
+    /* Other notifications, 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..8cc990a702 100644
--- a/dlls/comctl32/tests/pager.c
+++ b/dlls/comctl32/tests/pager.c
@@ -30,6 +30,107 @@
 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 empty_w[] = {0};
+static CHAR empty_a[] = {0};
+static CHAR large_a[] = "You should have received a copy of the GNU Lesser General Public License along with this ...";
+static WCHAR large_truncated_65_w[65] =
+{
+    'Y', 'o', 'u', ' ', 's', 'h', 'o', 'u', 'l', 'd', ' ', 'h', 'a', 'v', 'e', ' ', 'r', 'e', 'c', 'e', 'i', 'v',
+    'e', 'd', ' ', 'a', ' ', 'c', 'o', 'p', 'y', ' ', 'o', 'f', ' ', 't', 'h', 'e', ' ', 'G', 'N', 'U', ' ', 'L',
+    'e', 's', 's', 'e', 'r', ' ', 'G', 'e', 'n', 'e', 'r', 'a', 'l', ' ', 'P', 'u', 'b', 'l', 'i', 'c', 0
+};
+static WCHAR buffer[64];
+
+/* Text field conversion test behavior flags. */
+enum test_conversion_flags
+{
+    CONVERT_SEND = 0x01,
+    NOT_CONVERT_SEND = 0x02,
+    CONVERT_RECEIVE = 0x04,
+    NOT_CONVERT_RECEIVE = 0x08,
+    NULL_SEND_EMPTY = 0x10,
+    NOT_NULL_SEND_EMPTY = 0x20
+};
+
+static struct notify_test_info
+{
+    UINT unicode;
+    UINT ansi;
+    UINT_PTR id_from;
+    HWND hwnd_from;
+    /* Whether parent received notification */
+    BOOL received;
+    UINT test_id;
+    UINT sub_test_id;
+    /* Text field conversion test behavior flag */
+    DWORD flags;
+} notify_test_info;
+
+struct notify_test_send_data
+{
+    /* Data sent to pager */
+    WCHAR *send_text;
+    INT send_text_size;
+    INT send_text_max;
+    /* Data expected by parent of pager */
+    void *expect_text;
+};
+
+struct notify_test_receive_data
+{
+    /* Data sent to pager */
+    WCHAR *send_text;
+    INT send_text_size;
+    INT send_text_max;
+    /* Data for parent to write */
+    CHAR *write_pointer;
+    CHAR *write_text;
+    INT write_text_size;
+    INT write_text_max;
+    /* Data when message returned */
+    void *return_text;
+    INT return_text_max;
+};
+
+struct generic_text_helper_para
+{
+    void *ptr;
+    size_t size;
+    UINT *mask;
+    UINT required_mask;
+    WCHAR **text;
+    INT *text_max;
+    UINT code_unicode;
+    UINT code_ansi;
+    DWORD flags;
+};
+
+static const struct notify_test_send_data test_convert_send_datas[] =
+{
+    {test_w, sizeof(test_w), ARRAY_SIZE(buffer), test_a}
+};
+
+static const struct notify_test_send_data test_not_convert_send_datas[] =
+{
+    {test_w, sizeof(test_w), ARRAY_SIZE(buffer), test_w}
+};
+
+static const struct notify_test_receive_data test_convert_receive_datas[] =
+{
+    {empty_w, sizeof(empty_w), ARRAY_SIZE(buffer), NULL, test_a, sizeof(test_a), -1, test_w, ARRAY_SIZE(buffer)},
+    {empty_w, sizeof(empty_w), ARRAY_SIZE(buffer), test_a, NULL, 0, -1, test_w, ARRAY_SIZE(buffer)},
+    {NULL, sizeof(empty_w), ARRAY_SIZE(buffer), test_a, NULL, 0, -1, NULL, ARRAY_SIZE(buffer)},
+    {empty_w, sizeof(empty_w), ARRAY_SIZE(buffer), large_a, NULL, 0, -1, large_truncated_65_w, ARRAY_SIZE(buffer)},
+    {empty_w, sizeof(empty_w), ARRAY_SIZE(buffer), empty_a, 0, 0, 1, empty_w, 1},
+};
+
+static const struct notify_test_receive_data test_not_convert_receive_datas[] =
+{
+    {empty_w, sizeof(empty_w), ARRAY_SIZE(buffer), NULL, test_a, sizeof(test_a), -1, test_a, ARRAY_SIZE(buffer)},
+    {empty_w, sizeof(empty_w), ARRAY_SIZE(buffer), test_a, NULL, 0, -1, test_a, ARRAY_SIZE(buffer)},
+};
 
 #define CHILD1_ID 1
 #define CHILD2_ID 2
@@ -419,6 +520,308 @@ static void test_wm_notifyformat(void)
     UnregisterClassW(class_w, GetModuleHandleW(NULL));
 }
 
+static void notify_generic_text_handler(CHAR **text, INT *text_max)
+{
+    switch (notify_test_info.test_id)
+    {
+    case CONVERT_SEND:
+    case NOT_CONVERT_SEND:
+    {
+        const struct notify_test_send_data *data =
+            (notify_test_info.test_id == CONVERT_SEND ? test_convert_send_datas : test_not_convert_send_datas)
+            + notify_test_info.sub_test_id;
+        if (notify_test_info.flags & CONVERT_SEND)
+            ok(!lstrcmpA(data->expect_text, *text), "Code 0x%08x test 0x%08x sub test %d expect %s, got %s\n",
+               notify_test_info.unicode, notify_test_info.test_id, notify_test_info.sub_test_id,
+               (CHAR *)data->expect_text, *text);
+        else
+            ok(!lstrcmpW((WCHAR *)data->expect_text, (WCHAR *)*text),
+               "Code 0x%08x test 0x%08x sub test %d expect %s, got %s\n", notify_test_info.unicode,
+               notify_test_info.test_id, notify_test_info.sub_test_id, wine_dbgstr_w((WCHAR *)data->expect_text),
+               wine_dbgstr_w((WCHAR *)*text));
+        if (text_max)
+            ok(*text_max == data->send_text_max, "Code 0x%08x test 0x%08x sub test %d expect %d, got %d\n",
+               notify_test_info.unicode, notify_test_info.test_id, notify_test_info.sub_test_id, data->send_text_max,
+               *text_max);
+        break;
+    }
+    case CONVERT_RECEIVE:
+    case NOT_CONVERT_RECEIVE:
+    {
+        const struct notify_test_receive_data *data =
+            (notify_test_info.test_id == CONVERT_RECEIVE ? test_convert_receive_datas : test_not_convert_receive_datas)
+            + notify_test_info.sub_test_id;
+        if (text_max)
+            ok(*text_max == data->send_text_max, "Code 0x%08x test 0x%08x sub test %d expect %d, got %d\n",
+               notify_test_info.unicode, notify_test_info.test_id, notify_test_info.sub_test_id, data->send_text_max,
+               *text_max);
+
+        if (data->write_text)
+            memcpy(*text, data->write_text, data->write_text_size);
+        else
+            *text = data->write_pointer;
+        if (text_max && data->write_text_max != -1) *text_max = data->write_text_max;
+        break;
+    }
+    case NULL_SEND_EMPTY:
+        ok(!lstrcmpA(*text, empty_a), "Code 0x%08x test 0x%08x sub test %d expect empty text, got %s\n",
+           notify_test_info.unicode, notify_test_info.test_id, notify_test_info.sub_test_id, *text);
+        break;
+    case NOT_NULL_SEND_EMPTY:
+        ok(!*text, "Code 0x%08x test 0x%08x sub test %d expect null text\n", notify_test_info.unicode,
+           notify_test_info.test_id, notify_test_info.sub_test_id);
+        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_info.unicode) break;
+        ok(!notify_test_info.received, "Extra notification received\n");
+
+        ok(wParam == notify_test_info.id_from, "Expect %ld, got %ld\n", notify_test_info.id_from, wParam);
+        ok(hdr->code == notify_test_info.ansi, "Expect 0x%08x, got 0x%08x\n", notify_test_info.ansi, hdr->code);
+        ok(hdr->idFrom == notify_test_info.id_from, "Expect %ld, got %ld\n", notify_test_info.id_from, wParam);
+        ok(hdr->hwndFrom == notify_test_info.hwnd_from, "Expect %p, got %p\n", notify_test_info.hwnd_from, hdr->hwndFrom);
+
+        if (hdr->code != notify_test_info.ansi)
+        {
+            skip("Notification code mismatch, skipping lParam check\n");
+            return 0;
+        }
+        switch (hdr->code)
+        {
+        /* Toolbar */
+        case TBN_SAVE:
+        {
+            NMTBSAVE *nmtbs = (NMTBSAVE *)hdr;
+            notify_generic_text_handler((CHAR **)&nmtbs->tbButton.iString, NULL);
+            break;
+        }
+        case TBN_RESTORE:
+        {
+            NMTBRESTORE *nmtbr = (NMTBRESTORE *)hdr;
+            notify_generic_text_handler((CHAR **)&nmtbr->tbButton.iString, NULL);
+            break;
+        }
+        case TBN_GETBUTTONINFOA:
+        {
+            NMTOOLBARA *nmtb = (NMTOOLBARA *)hdr;
+            notify_generic_text_handler(&nmtb->pszText, &nmtb->cchText);
+            break;
+        }
+        case TBN_GETDISPINFOW:
+        {
+            NMTBDISPINFOW *nmtbdi = (NMTBDISPINFOW *)hdr;
+            notify_generic_text_handler((CHAR **)&nmtbdi->pszText, &nmtbdi->cchText);
+            break;
+        }
+        case TBN_GETINFOTIPA:
+        {
+            NMTBGETINFOTIPA *nmtbgit = (NMTBGETINFOTIPA *)hdr;
+            notify_generic_text_handler(&nmtbgit->pszText, &nmtbgit->cchTextMax);
+            break;
+        }
+        default:
+            ok(0, "Unexpected message 0x%08x\n", hdr->code);
+        }
+        notify_test_info.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)
+{
+    NMHDR *hdr = (NMHDR *)lParam;
+
+    notify_test_info.unicode = unicode;
+    notify_test_info.id_from = 1;
+    notify_test_info.hwnd_from = child1_wnd;
+    notify_test_info.ansi = ansi;
+    notify_test_info.received = FALSE;
+
+    hdr->code = unicode;
+    hdr->idFrom = 1;
+    hdr->hwndFrom = child1_wnd;
+
+    SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam);
+    ok(notify_test_info.received, "Expect notification received\n");
+    ok(hdr->code == code_change ? ansi : unicode, "Expect 0x%08x, got 0x%08x\n", hdr->code,
+       code_change ? ansi : unicode);
+}
+
+#define send_notify(a, b, c, d) _send_notify(pager, a, b, c, d)
+
+/* Send notify to test text field conversion. In parent proc notify_generic_text_handler() handles these messages */
+static void test_notify_generic_text_helper(HWND pager, void *ptr, size_t size, UINT *mask, UINT required_mask,
+                                            WCHAR **text, INT *text_max, UINT code_unicode, UINT code_ansi, DWORD flags)
+{
+    const struct notify_test_send_data *send_datas;
+    const struct notify_test_receive_data *receive_datas;
+    INT array_size;
+    INT i;
+
+    notify_test_info.flags = flags;
+
+    if (flags & (CONVERT_SEND | NOT_CONVERT_SEND))
+    {
+        if(flags & CONVERT_SEND)
+        {
+            notify_test_info.test_id = CONVERT_SEND;
+            send_datas = test_convert_send_datas;
+            array_size = ARRAY_SIZE(test_convert_send_datas);
+        }
+        else
+        {
+            notify_test_info.test_id = NOT_CONVERT_SEND;
+            send_datas = test_not_convert_send_datas;
+            array_size = ARRAY_SIZE(test_not_convert_send_datas);
+        }
+
+        for (i = 0; i < array_size; i++)
+        {
+            const struct notify_test_send_data *data = send_datas + i;
+            notify_test_info.sub_test_id = i;
+
+            memset(ptr, 0, size);
+            if (mask) *mask = required_mask;
+            if (data->send_text)
+            {
+                memcpy(buffer, data->send_text, data->send_text_size);
+                *text = buffer;
+            }
+            if (text_max) *text_max = data->send_text_max;
+            send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE);
+        }
+    }
+
+    if (flags & (CONVERT_RECEIVE | NOT_CONVERT_RECEIVE))
+    {
+        if (flags & CONVERT_RECEIVE)
+        {
+            notify_test_info.test_id = CONVERT_RECEIVE;
+            receive_datas = test_convert_receive_datas;
+            array_size = ARRAY_SIZE(test_convert_receive_datas);
+        }
+        else
+        {
+            notify_test_info.test_id = NOT_CONVERT_RECEIVE;
+            receive_datas = test_not_convert_receive_datas;
+            array_size = ARRAY_SIZE(test_not_convert_receive_datas);
+        }
+
+        for (i = 0; i < array_size; i++)
+        {
+            const struct notify_test_receive_data *data = receive_datas + i;
+            notify_test_info.sub_test_id = i;
+
+            memset(ptr, 0, size);
+            if (mask) *mask = required_mask;
+            if (data->send_text)
+            {
+                memcpy(buffer, data->send_text, data->send_text_size);
+                *text = buffer;
+            }
+            if (text_max) *text_max = data->send_text_max;
+            send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE);
+            if (data->return_text)
+            {
+                if (flags & CONVERT_RECEIVE)
+                    ok(!lstrcmpW(data->return_text, *text), "Code 0x%08x sub test %d expect %s, got %s\n", code_unicode,
+                       i, wine_dbgstr_w((WCHAR *)data->return_text), wine_dbgstr_w(*text));
+                else
+                    ok(!lstrcmpA(data->return_text, (CHAR *)*text), "Code 0x%08x sub test %d expect %s, got %s\n",
+                       code_unicode, i, (CHAR *)data->return_text, (CHAR *)*text);
+            }
+            if (text_max)
+                ok(data->return_text_max == *text_max, "Code 0x%08x sub test %d expect %d, got %d\n", code_unicode, i,
+                   data->return_text_max, *text_max);
+        }
+    }
+
+    /* Extra tests for other behavior flags that are not worth it to create their own test arrays */
+    memset(ptr, 0, size);
+    if (mask) *mask = required_mask;
+    if (text_max) *text_max = 1;
+    if(flags & NULL_SEND_EMPTY)
+        notify_test_info.test_id = NULL_SEND_EMPTY;
+    else
+        notify_test_info.test_id = NOT_NULL_SEND_EMPTY;
+    send_notify(code_unicode, code_ansi, (LPARAM)ptr, TRUE);
+}
+
+static void test_wm_notify(void)
+{
+    static const CHAR *class = "Pager notify class";
+    HWND parent, pager;
+    /* Tool Bar */
+    static NMTBRESTORE nmtbr;
+    static NMTBSAVE nmtbs;
+    static NMTOOLBARW nmtb;
+    static NMTBDISPINFOW nmtbdi;
+    static NMTBGETINFOTIPW nmtbgit;
+    static const struct generic_text_helper_para paras[] =
+    {
+        /* Tool Bar */
+        {&nmtbs, sizeof(nmtbs), NULL, 0, (WCHAR **)&nmtbs.tbButton.iString, NULL, TBN_SAVE, TBN_SAVE,
+         NOT_CONVERT_SEND | NOT_CONVERT_RECEIVE},
+        {&nmtbr, sizeof(nmtbr), NULL, 0, (WCHAR **)&nmtbr.tbButton.iString, NULL, TBN_RESTORE, TBN_RESTORE,
+         NOT_CONVERT_SEND | NOT_CONVERT_RECEIVE},
+        {&nmtbdi, sizeof(nmtbdi), &nmtbdi.dwMask, TBNF_TEXT, &nmtbdi.pszText, &nmtbdi.cchText, TBN_GETDISPINFOW,
+         TBN_GETDISPINFOW, NOT_CONVERT_SEND | NOT_CONVERT_RECEIVE},
+        {&nmtb, sizeof(nmtb), NULL, 0, &nmtb.pszText, &nmtb.cchText, TBN_GETBUTTONINFOW, TBN_GETBUTTONINFOA,
+         NULL_SEND_EMPTY | CONVERT_SEND | CONVERT_RECEIVE},
+        {&nmtbgit, sizeof(nmtbgit), NULL, 0, &nmtbgit.pszText, &nmtbgit.cchTextMax, TBN_GETINFOTIPW, TBN_GETINFOTIPA,
+         NOT_CONVERT_SEND | CONVERT_RECEIVE}
+    };
+    INT i;
+
+    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);
+
+    for (i = 0; i < ARRAY_SIZE(paras); i++)
+    {
+        const struct generic_text_helper_para *p = paras + i;
+        test_notify_generic_text_helper(pager, p->ptr, p->size, p->mask, p->required_mask, p->text, p->text_max,
+                                        p->code_unicode, p->code_ansi, p->flags);
+    }
+
+    DestroyWindow(parent);
+    UnregisterClassA(class, GetModuleHandleA(NULL));
+}
+
 static void init_functions(void)
 {
     HMODULE mod = LoadLibraryA("comctl32.dll");
@@ -447,6 +850,7 @@ START_TEST(pager)
 
     test_pager();
     test_wm_notifyformat();
+    test_wm_notify();
 
     DestroyWindow(parent_wnd);
 }
-- 
2.18.0

[Nikolay Sivov]

On 09/11/2018 03:26 PM, Zhiyi Zhang wrote:
> +/* Text field conversion test behavior flags. */
> +enum test_conversion_flags
> +{
> +    CONVERT_SEND = 0x01,
> +    NOT_CONVERT_SEND = 0x02,
> +    CONVERT_RECEIVE = 0x04,
> +    NOT_CONVERT_RECEIVE = 0x08,
> +    NULL_SEND_EMPTY = 0x10,
> +    NOT_NULL_SEND_EMPTY = 0x20
> +};
> I think those should be called DONT_*, or changed to SEND_NOT_CONVERTED, 
> or DONT_CONVERT_ON_SEND. or similar. And null case something like 
> SEND_EMPTY_IF_NULL,
> otherwise it's hard to understand without looking at what it does. By 
> the way, why is it necessary to have both flags in tests and not in 
> implementation?
> +/* Send notify to test text field conversion. In parent proc notify_generic_text_handler() handles these messages */
> +static void test_notify_generic_text_helper(HWND pager, void *ptr, size_t size, UINT *mask, UINT required_mask,
> +                                            WCHAR **text, INT *text_max, UINT code_unicode, UINT code_ansi, DWORD flags)
> +{
> +    const struct notify_test_send_data *send_datas;
> +    const struct notify_test_receive_data *receive_datas;
> I don't think 'datas' is the word, you can drop _data from structure 
> names, and use send_data for variable.
> +static void _send_notify(HWND pager, UINT unicode, UINT ansi, LPARAM lParam, BOOL code_change)
> +{
> +    NMHDR *hdr = (NMHDR *)lParam;
> +
> +    notify_test_info.unicode = unicode;
> +    notify_test_info.id_from = 1;
> +    notify_test_info.hwnd_from = child1_wnd;
> +    notify_test_info.ansi = ansi;
> +    notify_test_info.received = FALSE;
> +
> +    hdr->code = unicode;
> +    hdr->idFrom = 1;
> +    hdr->hwndFrom = child1_wnd;
> +
> +    SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam);
> +    ok(notify_test_info.received, "Expect notification received\n");
> +    ok(hdr->code == code_change ? ansi : unicode, "Expect 0x%08x, got 0x%08x\n", hdr->code,
> +       code_change ? ansi : unicode);
> +}
> +
> +#define send_notify(a, b, c, d) _send_notify(pager, a, b, c, d)

Saving one argument is not worth it, it's usually done like that if you 
want to pass line number to ok_(), otherwise I don't see a point.
> +    for (i = 0; i < ARRAY_SIZE(paras); i++)
> +    {
> +        const struct generic_text_helper_para *p = paras + i;
> +        test_notify_generic_text_helper(pager, p->ptr, p->size, p->mask, p->required_mask, p->text, p->text_max,
> +                                        p->code_unicode, p->code_ansi, p->flags);
> +    }
> Is it possible to pass structure pointer here?

[Zhiyi Zhang]

On 09/13/2018 04:57 PM, Nikolay Sivov wrote:
> On 09/11/2018 03:26 PM, Zhiyi Zhang wrote:
> > +/* Text field conversion test behavior flags. */
> > +enum test_conversion_flags
> > +{
> > +    CONVERT_SEND = 0x01,
> > +    NOT_CONVERT_SEND = 0x02,
> > +    CONVERT_RECEIVE = 0x04,
> > +    NOT_CONVERT_RECEIVE = 0x08,
> > +    NULL_SEND_EMPTY = 0x10,
> > +    NOT_NULL_SEND_EMPTY = 0x20
> > +};
> > I think those should be called DONT_*, or changed to SEND_NOT_CONVERTED, or DONT_CONVERT_ON_SEND. or similar. And null case something like SEND_EMPTY_IF_NULL,
> > otherwise it's hard to understand without looking at what it does. By the way, why is it necessary to have both flags in tests and not in implementation?
> > Because there are some cases where I don't want to test it at all, for example, notification that have a constant text fields.
> > If no CONVERT_RECEIVE and NOT_CONVERT_RECEIVE, we can just skip the such tests to avoid writing such constant text fields.
> > As for implementation, if it's NOT_*, we just skip the process.
> > +/* Send notify to test text field conversion. In parent proc notify_generic_text_handler() handles these messages */
> > +static void test_notify_generic_text_helper(HWND pager, void *ptr, size_t size, UINT *mask, UINT required_mask,
> > +                                            WCHAR **text, INT *text_max, UINT code_unicode, UINT code_ansi, DWORD flags)
> > +{
> > +    const struct notify_test_send_data *send_datas;
> > +    const struct notify_test_receive_data *receive_datas;
> > I don't think 'datas' is the word, you can drop _data from structure names, and use send_data for variable.
> > +static void _send_notify(HWND pager, UINT unicode, UINT ansi, LPARAM lParam, BOOL code_change)
> > +{
> > +    NMHDR *hdr = (NMHDR *)lParam;
> > +
> > +    notify_test_info.unicode = unicode;
> > +    notify_test_info.id_from = 1;
> > +    notify_test_info.hwnd_from = child1_wnd;
> > +    notify_test_info.ansi = ansi;
> > +    notify_test_info.received = FALSE;
> > +
> > +    hdr->code = unicode;
> > +    hdr->idFrom = 1;
> > +    hdr->hwndFrom = child1_wnd;
> > +
> > +    SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam);
> > +    ok(notify_test_info.received, "Expect notification received\n");
> > +    ok(hdr->code == code_change ? ansi : unicode, "Expect 0x%08x, got 0x%08x\n", hdr->code,
> > +       code_change ? ansi : unicode);
> > +}
> > +
> > +#define send_notify(a, b, c, d) _send_notify(pager, a, b, c, d)
>
> Saving one argument is not worth it, it's usually done like that if you want to pass line number to ok_(), otherwise I don't see a point.
> > +    for (i = 0; i < ARRAY_SIZE(paras); i++)
> > +    {
> > +        const struct generic_text_helper_para *p = paras + i;
> > +        test_notify_generic_text_helper(pager, p->ptr, p->size, p->mask, p->required_mask, p->text, p->text_max,
> > +                                        p->code_unicode, p->code_ansi, p->flags);
> > +    }
> > Is it possible to pass structure pointer here?

[Zhiyi Zhang]

On 09/13/2018 04:57 PM, Nikolay Sivov wrote:
> On 09/11/2018 03:26 PM, Zhiyi Zhang wrote:
> > +/* Text field conversion test behavior flags. */
> > +enum test_conversion_flags
> > +{
> > +    CONVERT_SEND = 0x01,
> > +    NOT_CONVERT_SEND = 0x02,
> > +    CONVERT_RECEIVE = 0x04,
> > +    NOT_CONVERT_RECEIVE = 0x08,
> > +    NULL_SEND_EMPTY = 0x10,
> > +    NOT_NULL_SEND_EMPTY = 0x20
> > +};
> > I think those should be called DONT_*, or changed to SEND_NOT_CONVERTED, or DONT_CONVERT_ON_SEND. or similar. And null case something like SEND_EMPTY_IF_NULL,
> > otherwise it's hard to understand without looking at what it does. By the way, why is it necessary to have both flags in tests and not in implementation?
> > Because there are some cases where I don't want to test it at all, for example, notification that have a constant text fields.
> > If no CONVERT_RECEIVE and NOT_CONVERT_RECEIVE, we can just skip the such tests to avoid writing such constant text fields.
> > As for implementation, if it's NOT_*, we just skip the process.
> > +/* Send notify to test text field conversion. In parent proc notify_generic_text_handler() handles these messages */
> > +static void test_notify_generic_text_helper(HWND pager, void *ptr, size_t size, UINT *mask, UINT required_mask,
> > +                                            WCHAR **text, INT *text_max, UINT code_unicode, UINT code_ansi, DWORD flags)
> > +{
> > +    const struct notify_test_send_data *send_datas;
> > +    const struct notify_test_receive_data *receive_datas;
> > I don't think 'datas' is the word, you can drop _data from structure names, and use send_data for variable.
> > +static void _send_notify(HWND pager, UINT unicode, UINT ansi, LPARAM lParam, BOOL code_change)
> > +{
> > +    NMHDR *hdr = (NMHDR *)lParam;
> > +
> > +    notify_test_info.unicode = unicode;
> > +    notify_test_info.id_from = 1;
> > +    notify_test_info.hwnd_from = child1_wnd;
> > +    notify_test_info.ansi = ansi;
> > +    notify_test_info.received = FALSE;
> > +
> > +    hdr->code = unicode;
> > +    hdr->idFrom = 1;
> > +    hdr->hwndFrom = child1_wnd;
> > +
> > +    SendMessageW(pager, WM_NOTIFY, hdr->idFrom, lParam);
> > +    ok(notify_test_info.received, "Expect notification received\n");
> > +    ok(hdr->code == code_change ? ansi : unicode, "Expect 0x%08x, got 0x%08x\n", hdr->code,
> > +       code_change ? ansi : unicode);
> > +}
> > +
> > +#define send_notify(a, b, c, d) _send_notify(pager, a, b, c, d)
>
> Saving one argument is not worth it, it's usually done like that if you want to pass line number to ok_(), otherwise I don't see a point.
> > +    for (i = 0; i < ARRAY_SIZE(paras); i++)
> > +    {
> > +        const struct generic_text_helper_para *p = paras + i;
> > +        test_notify_generic_text_helper(pager, p->ptr, p->size, p->mask, p->required_mask, p->text, p->text_max,
> > +                                        p->code_unicode, p->code_ansi, p->flags);
> > +    }
> > Is it possible to pass structure pointer here?