On 8/28/2018 6:27 AM, Zhiyi Zhang wrote:
Fix BibleWorks 10 not displaying toolbar.
Signed-off-by: Zhiyi Zhang zzhang@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);elseexpect_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);
}