Currently it doesn't work as expected, namely gets stuck on delimiter and caret is moved before the delimiter.
Unnecessary more robust than Windows, which feels like doesn't do any checks at all.
EDIT1: v2: return length in `WB_RIGHT` if its bigger than default return value of 0 on invalid values
-- v2: comctl32: Fix PathWordBreakProc.
From: Vladislav Timonin timoninvlad@yandex.ru
--- dlls/comctl32/commctrl.c | 53 ++++++++++-- dlls/comctl32/tests/combo.c | 159 ++++++++++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 8 deletions(-)
diff --git a/dlls/comctl32/commctrl.c b/dlls/comctl32/commctrl.c index c5910b40869..18ff9e41bff 100644 --- a/dlls/comctl32/commctrl.c +++ b/dlls/comctl32/commctrl.c @@ -1628,15 +1628,52 @@ static inline BOOL IsDelimiter(WCHAR c)
static int CALLBACK PathWordBreakProc(LPCWSTR lpch, int ichCurrent, int cch, int code) { - if (code == WB_ISDELIMITER) - return IsDelimiter(lpch[ichCurrent]); - else - { - int dir = (code == WB_LEFT) ? -1 : 1; - for(; 0 <= ichCurrent && ichCurrent < cch; ichCurrent += dir) - if (IsDelimiter(lpch[ichCurrent])) return ichCurrent; + INT ret = 0; + + switch (code) { + case WB_LEFT: + if (!lpch || ichCurrent < 0 || ichCurrent > cch) + break; + + /* skip trailing delimiters */ + while (ichCurrent && IsDelimiter(lpch[ichCurrent - 1])) + ichCurrent--; + /* skip word */ + while (ichCurrent && !IsDelimiter(lpch[ichCurrent - 1])) + ichCurrent--; + /* special case: if there's a single leading delimiter, skip it */ + if (ichCurrent == 1 && IsDelimiter(lpch[ichCurrent - 1])) + ichCurrent--; + + ret = ichCurrent; + break; + case WB_RIGHT: + if (!lpch || ichCurrent < 0 || ichCurrent > cch) + { + if (cch > ret) + ret = cch; + break; + } + + /* skip word */ + while (ichCurrent < cch && lpch[ichCurrent] && !IsDelimiter(lpch[ichCurrent])) + ichCurrent++; + /* move caret after the delimiters */ + while (ichCurrent < cch && lpch[ichCurrent] && IsDelimiter(lpch[ichCurrent])) + ichCurrent++; + + ret = ichCurrent; + break; + case WB_ISDELIMITER: + if (lpch && ichCurrent >= 0 && ichCurrent < cch) + ret = IsDelimiter(lpch[ichCurrent]); + break; + default: + ERR("unexpected action code\n"); + break; } - return ichCurrent; + + return ret; }
/*********************************************************************** diff --git a/dlls/comctl32/tests/combo.c b/dlls/comctl32/tests/combo.c index 3eceeb55bd5..90522df88f2 100644 --- a/dlls/comctl32/tests/combo.c +++ b/dlls/comctl32/tests/combo.c @@ -1514,6 +1514,164 @@ static void test_comboex_CBEN_GETDISPINFO(void) DestroyWindow(combo); }
+static BOOL hold_key(int vk) +{ + BYTE key_state[256]; + BOOL result; + + result = GetKeyboardState(key_state); + ok(result, "GetKeyboardState failed.\n"); + if (!result) return FALSE; + key_state[vk] |= 0x80; + result = SetKeyboardState(key_state); + ok(result, "SetKeyboardState failed.\n"); + return result != 0; +} + +static BOOL release_key(int vk) +{ + BYTE key_state[256]; + BOOL result; + + result = GetKeyboardState(key_state); + ok(result, "GetKeyboardState failed.\n"); + if (!result) return FALSE; + key_state[vk] &= ~0x80; + result = SetKeyboardState(key_state); + ok(result, "SetKeyboardState failed.\n"); + return result != 0; +} + +static LRESULT send_ctrl_key(HWND hwnd, UINT key) +{ + LRESULT result; + hold_key(VK_CONTROL); + result = SendMessageA(hwnd, WM_KEYDOWN, key, 1); + release_key(VK_CONTROL); + return result; +} + +static void move_word(INT line, HWND edit, UINT direction, INT expected_caret_position) +{ + INT selection_start = -1, selection_end = -1; + + send_ctrl_key(edit, direction); + SendMessageA(edit, EM_GETSEL, (WPARAM)&selection_start, (WPARAM)&selection_end); + ok_(__FILE__, line)(selection_start == expected_caret_position, "Unexpected caret position %i.\n", selection_start); +} + +static void test_comboex_CBES_EX_PATHWORDBREAKPROC(void) +{ + HWND combo, edit; + INT start, end; + WCHAR some_pathW[] = L"some/path"; + WCHAR delimitersW[] = L"a .\/"; + EDITWORDBREAKPROCW work_break_proc; + int ret; + + combo = createComboEx(WS_VISIBLE | WS_CHILD | CBS_DROPDOWN); + SendMessageA(combo, CBEM_SETEXTENDEDSTYLE, 0, CBES_EX_PATHWORDBREAKPROC); + edit = (HWND)SendMessageA(combo, CBEM_GETEDITCONTROL, 0, 0); + + /* multiple delimiters */ + SetWindowTextA(edit, "C:\aa bb\cc.dd/ee\ff\/ ."); + /* from left to right */ + SendMessageA(edit, EM_GETSEL, (WPARAM)&start, (WPARAM)&end); + ok(start == 0, "Unexpected caret position %i.\n", start); /* |C:\aa bb\cc.dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_RIGHT, 3); /* C:|aa bb\cc.dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_RIGHT, 6); /* C:\aa |bb\cc.dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_RIGHT, 9); /* C:\aa bb|cc.dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_RIGHT, 12); /* C:\aa bb\cc.|dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_RIGHT, 15); /* C:\aa bb\cc.dd/|ee\ff/ . */ + move_word(__LINE__, edit, VK_RIGHT, 18); /* C:\aa bb\cc.dd/ee|ff/ . */ + move_word(__LINE__, edit, VK_RIGHT, 24); /* C:\aa bb\cc.dd/ee\ff/ .| */ + /* from right to left */ + SendMessageA(edit, EM_SETSEL, GetWindowTextLengthA(edit), GetWindowTextLengthA(edit)); + SendMessageA(edit, EM_GETSEL, (WPARAM)&start, (WPARAM)&end); + ok(start == 24, "Unexpected caret position %i.\n", start); /* C:\aa bb\cc.dd/ee\ff/ .| */ + move_word(__LINE__, edit, VK_LEFT, 18); /* C:\aa bb\cc.dd/ee|ff/ . */ + move_word(__LINE__, edit, VK_LEFT, 15); /* C:\aa bb\cc.dd/|ee\ff/ . */ + move_word(__LINE__, edit, VK_LEFT, 12); /* C:\aa bb\cc.|dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_LEFT, 9); /* C:\aa bb|cc.dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_LEFT, 6); /* C:\aa |bb\cc.dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_LEFT, 3); /* C:|aa bb\cc.dd/ee\ff/ . */ + move_word(__LINE__, edit, VK_LEFT, 0); /* |C:\aa bb\cc.dd/ee\ff/ . */ + + /* no trailing delimiter and leading delimiter */ + SetWindowTextA(edit, "\\some\path"); + SendMessageA(edit, EM_SETSEL, GetWindowTextLengthA(edit), GetWindowTextLengthA(edit)); + SendMessageA(edit, EM_GETSEL, (WPARAM)&start, (WPARAM)&end); + ok(start == 11, "Unexpected caret position %i.\n", start); /* \some\path| */ + move_word(__LINE__, edit, VK_LEFT, 7); /* \some|path */ + move_word(__LINE__, edit, VK_LEFT, 2); /* \|some\path*/ + move_word(__LINE__, edit, VK_LEFT, 0); /* |\some\path*/ + + /* single leading delimiter */ + SetWindowTextA(edit, "/some/path"); + SendMessageA(edit, EM_SETSEL, GetWindowTextLengthA(edit), GetWindowTextLengthA(edit)); + SendMessageA(edit, EM_GETSEL, (WPARAM)&start, (WPARAM)&end); + ok(start == 10, "Unexpected caret position %i.\n", start); /* /some/path|*/ + move_word(__LINE__, edit, VK_LEFT, 6); /* /some/|path*/ + move_word(__LINE__, edit, VK_LEFT, 0); /* |/some/path*/ + + work_break_proc = (EDITWORDBREAKPROCW)SendMessageA(edit, EM_GETWORDBREAKPROC, 0, 0); + + /* null text */ + ret = work_break_proc(NULL, 0, 0, WB_LEFT); + ok(ret == 0, "Unexpected return value %i.\n", ret); + if (!strcmp(winetest_platform, "wine")) /* crashes on Windows */ + { + ret = work_break_proc(NULL, 0, 0, WB_RIGHT); + ok(ret == 0, "Unexpected return value %i.\n", ret); + ret = work_break_proc(NULL, 0, 0, WB_ISDELIMITER); + ok(ret == 0, "Unexpected return value %i.\n", ret); + ret = work_break_proc(NULL, 10, 10, WB_LEFT); + ok(ret == 0, "Unexpected return value %i.\n", ret); + ret = work_break_proc(NULL, 10, 10, WB_RIGHT); + ok(ret == 10, "Unexpected return value %i.\n", ret); + ret = work_break_proc(NULL, 10, 10, WB_ISDELIMITER); + ok(ret == 0, "Unexpected return value %i.\n", ret); + } + + ret = work_break_proc(some_pathW, 3, 0, WB_LEFT); + ok(ret == 0, "Unexpected return value %i.\n", ret); + ret = work_break_proc(some_pathW, lstrlenW(some_pathW), lstrlenW(some_pathW), WB_LEFT); + ok(ret == 5, "Unexpected return value %i.\n", ret); + ret = work_break_proc(some_pathW, lstrlenW(some_pathW), lstrlenW(some_pathW), WB_RIGHT); + ok(ret == lstrlenW(some_pathW), "Unexpected return value %i.\n", ret); + if (!strcmp(winetest_platform, "wine")) + { + ret = work_break_proc(some_pathW, 15, 5, WB_LEFT); /* doesn't always return the same value */ + ok(ret == 0, "Unexpected return value %i.\n", ret); + ret = work_break_proc(some_pathW, -10, 0, WB_LEFT); /* gets stuck in a loop */ + ok(ret == 0, "Unexpected return value %i.\n", ret); + ret = work_break_proc(some_pathW, 15, 5, WB_RIGHT); /* crash or unexpected value */ + ok(ret == 5, "Unexpected return value %i.\n", ret); + ret = work_break_proc(some_pathW, 0, -5, WB_RIGHT); /* crash or unexpected value */ + ok(ret == 0, "Unexpected return value %i.\n", ret); + } + + /* invalid action code */ + ret = work_break_proc(NULL, 5, 5, 0xFFFF); + ok(ret == 0, "Unexpected return value %i.\n", ret); + + /* delimiter */ + ret = work_break_proc(delimitersW, -10, lstrlenW(delimitersW), WB_ISDELIMITER); + ok(ret == 0, "Unexpected return value %i.\n", ret); + ret = work_break_proc(delimitersW, 0, lstrlenW(delimitersW), WB_ISDELIMITER); + ok(ret == 0, "Unexpected return value %i.\n", ret); + ret = work_break_proc(delimitersW, 1, lstrlenW(delimitersW), WB_ISDELIMITER); + ok(ret == 1, "Unexpected return value %i.\n", ret); + ret = work_break_proc(delimitersW, 2, lstrlenW(delimitersW), WB_ISDELIMITER); + ok(ret == 1, "Unexpected return value %i.\n", ret); + ret = work_break_proc(delimitersW, 3, lstrlenW(delimitersW), WB_ISDELIMITER); + ok(ret == 1, "Unexpected return value %i.\n", ret); + ret = work_break_proc(delimitersW, 4, lstrlenW(delimitersW), WB_ISDELIMITER); + ok(ret == 1, "Unexpected return value %i.\n", ret); + ret = work_break_proc(delimitersW, 10, lstrlenW(delimitersW), WB_ISDELIMITER); + ok(ret == 0, "Unexpected return value %i.\n", ret); +} + START_TEST(combo) { ULONG_PTR ctx_cookie; @@ -1534,6 +1692,7 @@ START_TEST(combo) test_comboex_subclass(); test_comboex_get_set_item(); test_comboex_CBEN_GETDISPINFO(); + test_comboex_CBES_EX_PATHWORDBREAKPROC();
if (!load_v6_module(&ctx_cookie, &hCtx)) {
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=128492
Your paranoid android.
=== w1064v1507 (32 bit report) ===
comctl32: combo.c:1672: Test failed: Unexpected return value 1.
-
Marvin -
Vladislav Timonin -
Vladislav Timonin (＠vt)