[PATCH 0/4] MR6304: comctl32/edit: Fix incorrect size for format rect when it is smaller than text.
From: Jacob Czekalla <jczekalla(a)codeweavers.com> --- dlls/user32/tests/edit.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index bbde9b16616..6a769368fe9 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -3426,6 +3426,42 @@ static void test_dbcs_WM_CHAR(void) } } +struct state_tests +{ + int style; + int style_ex; + RECT input; +}; + +static void test_small_rect(void) +{ + struct state_tests tests[6]; + HWND edit; + RECT correct; + RECT rc; + + tests[0] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}}; + tests[1] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 10, 10}}; + tests[2] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 10}}; + tests[3] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 0, 0}}; + tests[4] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 10, 10}}; + tests[5] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 10}}; + + for (int i = 0; i < ARRAY_SIZE(tests); i++) + { + edit = create_editcontrol(tests[i].style, tests[i].style_ex); + SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&correct); + SetWindowTextA(edit, "Test Test Test\r\n\r\nTest Test Test Test Test Test Test Test Test Test Test Test\r\n\r\nTest Test Test"); + SendMessageA(edit, EM_SETRECT, 0, (LPARAM)&tests[i].input); + SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&rc); + todo_wine ok(rc.left == correct.left, "left(%d): Expected %ld, got %ld\n", i, correct.left, rc.left); + todo_wine ok(rc.top == correct.top, "top(%d): Expected %ld, got %ld\n", i, correct.top, rc.top); + todo_wine ok(rc.right == correct.right, "right(%d): Expected %ld, got %ld\n", i, correct.right, rc.right); + todo_wine ok(rc.bottom == correct.bottom, "bottom(%d): Expected %ld, got %ld\n", i, correct.bottom, rc.bottom); + DestroyWindow(edit); + } +} + START_TEST(edit) { BOOL b; @@ -3464,6 +3500,7 @@ START_TEST(edit) test_EM_GETLINE(); test_wordbreak_proc(); test_dbcs_WM_CHAR(); + test_small_rect(); UnregisterWindowClasses(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
From: Jacob Czekalla <jczekalla(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56873 --- dlls/user32/edit.c | 25 ++++++++++++++++++++++++- dlls/user32/tests/edit.c | 8 ++++---- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/dlls/user32/edit.c b/dlls/user32/edit.c index 39f429813d6..a57defea971 100644 --- a/dlls/user32/edit.c +++ b/dlls/user32/edit.c @@ -2307,6 +2307,26 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) EDIT_SetCaretPos(es, es->selection_end, es->flags & EF_AFTER_WRAP); } +static int EDIT_is_valid_format_rect(EDITSTATE *es, const RECT *rc) +{ + RECT edit_rect; + int text_height = es->line_height * es->line_count; + + GetClientRect(es->hwndSelf, &edit_rect); + if (rc->bottom == 0 && rc->top == 0 && rc->right == 0 && rc->left == 0) + return 0; + if (rc->top >= rc->bottom) + return 0; + if (rc->left >= rc->right) + return 0; + if (text_height > (rc->bottom - rc->top)) + return 0; + if (es->text_width > (rc->right - rc->left)) + return 0; + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) + return 0; + return 1; +} /********************************************************************* * @@ -2322,7 +2342,10 @@ static void EDIT_SetRectNP(EDITSTATE *es, const RECT *rc) INT bw, bh; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - CopyRect(&es->format_rect, rc); + if (!EDIT_is_valid_format_rect(es, rc)) + GetClientRect(es->hwndSelf, &es->format_rect); + else + CopyRect(&es->format_rect, rc); if (ExStyle & WS_EX_CLIENTEDGE) { es->format_rect.left++; diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index 6a769368fe9..b92c40af3d8 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -3454,10 +3454,10 @@ static void test_small_rect(void) SetWindowTextA(edit, "Test Test Test\r\n\r\nTest Test Test Test Test Test Test Test Test Test Test Test\r\n\r\nTest Test Test"); SendMessageA(edit, EM_SETRECT, 0, (LPARAM)&tests[i].input); SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&rc); - todo_wine ok(rc.left == correct.left, "left(%d): Expected %ld, got %ld\n", i, correct.left, rc.left); - todo_wine ok(rc.top == correct.top, "top(%d): Expected %ld, got %ld\n", i, correct.top, rc.top); - todo_wine ok(rc.right == correct.right, "right(%d): Expected %ld, got %ld\n", i, correct.right, rc.right); - todo_wine ok(rc.bottom == correct.bottom, "bottom(%d): Expected %ld, got %ld\n", i, correct.bottom, rc.bottom); + ok(rc.left == correct.left, "left(%d): Expected %ld, got %ld\n", i, correct.left, rc.left); + ok(rc.top == correct.top, "top(%d): Expected %ld, got %ld\n", i, correct.top, rc.top); + ok(rc.right == correct.right, "right(%d): Expected %ld, got %ld\n", i, correct.right, rc.right); + ok(rc.bottom == correct.bottom, "bottom(%d): Expected %ld, got %ld\n", i, correct.bottom, rc.bottom); DestroyWindow(edit); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
From: Jacob Czekalla <jczekalla(a)codeweavers.com> --- dlls/comctl32/tests/edit.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index bce0215ef19..01f46073a5a 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -3791,6 +3791,42 @@ static void test_ime(void) DestroyWindow(hwnd); } +struct state_tests +{ + int style; + int style_ex; + RECT input; +}; + +static void test_small_rect(void) +{ + struct state_tests tests[6]; + HWND edit; + RECT correct; + RECT rc; + + tests[0] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}}; + tests[1] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 10, 10}}; + tests[2] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 10}}; + tests[3] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 0, 0}}; + tests[4] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 10, 10}}; + tests[5] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 10}}; + + for (int i = 0; i < ARRAY_SIZE(tests); i++) + { + edit = create_editcontrol(tests[i].style, tests[i].style_ex); + SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&correct); + SetWindowTextA(edit, "Test Test Test\r\n\r\nTest Test Test Test Test Test Test Test Test Test Test Test\r\n\r\nTest Test Test"); + SendMessageA(edit, EM_SETRECT, 0, (LPARAM)&tests[i].input); + SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&rc); + ok(rc.left == correct.left, "left(%d): Expected %ld, got %ld\n", i, correct.left, rc.left); + ok(rc.top == correct.top, "top(%d): Expected %ld, got %ld\n", i, correct.top, rc.top); + ok(rc.right == correct.right, "right(%d): Expected %ld, got %ld\n", i, correct.right, rc.right); + ok(rc.bottom == correct.bottom, "bottom(%d): Expected %ld, got %ld\n", i, correct.bottom, rc.bottom); + DestroyWindow(edit); + } +} + START_TEST(edit) { ULONG_PTR ctx_cookie; @@ -3838,6 +3874,7 @@ START_TEST(edit) test_change_focus(); test_cue_banner(); test_ime(); + test_small_rect(); UnregisterWindowClasses(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
From: Jacob Czekalla <jczekalla(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56873 --- dlls/comctl32/edit.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c index 4b950412617..3d9def178bc 100644 --- a/dlls/comctl32/edit.c +++ b/dlls/comctl32/edit.c @@ -2232,6 +2232,26 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) EDIT_SetCaretPos(es, es->selection_end, es->flags & EF_AFTER_WRAP); } +static int EDIT_is_valid_format_rect(EDITSTATE *es, const RECT *rc) +{ + RECT edit_rect; + int text_height = es->line_height * es->line_count; + + GetClientRect(es->hwndSelf, &edit_rect); + if (rc->bottom == 0 && rc->top == 0 && rc->right == 0 && rc->left == 0) + return 0; + if (rc->top >= rc->bottom) + return 0; + if (rc->left >= rc->right) + return 0; + if (text_height > (rc->bottom - rc->top)) + return 0; + if (es->text_width > (rc->right - rc->left)) + return 0; + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) + return 0; + return 1; +} /********************************************************************* * @@ -2247,7 +2267,10 @@ static void EDIT_SetRectNP(EDITSTATE *es, const RECT *rc) INT bw, bh; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - CopyRect(&es->format_rect, rc); + if (!EDIT_is_valid_format_rect(es, rc)) + GetClientRect(es->hwndSelf, &es->format_rect); + else + CopyRect(&es->format_rect, rc); if (ExStyle & WS_EX_CLIENTEDGE) { es->format_rect.left++; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
} }
+struct state_tests +{ + int style; + int style_ex; + RECT input; +}; + +static void test_small_rect(void) +{ + struct state_tests tests[6];
You can declare the test data like this ``` static const struct { DWORD style; DWORD ex_style; RECT rect; } tests[] = { {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}}, {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 10, 10}}, ... }; ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79091
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
+ + tests[0] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}}; + tests[1] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 10, 10}}; + tests[2] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 10}}; + tests[3] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 0, 0}}; + tests[4] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 10, 10}}; + tests[5] = (struct state_tests){ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 10}}; + + for (int i = 0; i < ARRAY_SIZE(tests); i++) + { + edit = create_editcontrol(tests[i].style, tests[i].style_ex); + SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&correct); + SetWindowTextA(edit, "Test Test Test\r\n\r\nTest Test Test Test Test Test Test Test Test Test Test Test\r\n\r\nTest Test Test"); + SendMessageA(edit, EM_SETRECT, 0, (LPARAM)&tests[i].input); + SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&rc); + todo_wine ok(rc.left == correct.left, "left(%d): Expected %ld, got %ld\n", i, correct.left, rc.left); Let's use EqualRect() instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79092
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
} }
+struct state_tests +{ + int style; + int style_ex; + RECT input; +}; + +static void test_small_rect(void) +{ + struct state_tests tests[6]; + HWND edit; + RECT correct; + RECT rc;
Let's write them as "RECT rect, expected_rect;" -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79093
Zhiyi Zhang (@zhiyi) commented about dlls/user32/edit.c:
EDIT_SetCaretPos(es, es->selection_end, es->flags & EF_AFTER_WRAP); }
+static int EDIT_is_valid_format_rect(EDITSTATE *es, const RECT *rc) +{ + RECT edit_rect; + int text_height = es->line_height * es->line_count; + + GetClientRect(es->hwndSelf, &edit_rect); + if (rc->bottom == 0 && rc->top == 0 && rc->right == 0 && rc->left == 0) + return 0; + if (rc->top >= rc->bottom) + return 0; + if (rc->left >= rc->right) + return 0; Are these three checks above necessary? I think they are covered by the following checks. If not, I think you can use IsRectEmpty().
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79094
Zhiyi Zhang (@zhiyi) commented about dlls/user32/edit.c:
+{ + RECT edit_rect; + int text_height = es->line_height * es->line_count; + + GetClientRect(es->hwndSelf, &edit_rect); + if (rc->bottom == 0 && rc->top == 0 && rc->right == 0 && rc->left == 0) + return 0; + if (rc->top >= rc->bottom) + return 0; + if (rc->left >= rc->right) + return 0; + if (text_height > (rc->bottom - rc->top)) + return 0; + if (es->text_width > (rc->right - rc->left)) + return 0; + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) Is this condition covered by your tests? What about (rc->right - rc->left) > (edit_rect.right - edit_rect.left)?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79095
participants (3)
-
Jacob Czekalla -
Jacob Czekalla (＠JacobCzekalla)
-
Zhiyi Zhang (＠zhiyi)