[PATCH v11 0/4] MR6304: comctl32/edit: Fix incorrect size for format rect.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56873 -- v11: comctl32/edit: Fix incorrect size for format rect when it is smaller than text. comctl32/tests: Add test for edit control format rect size. user32/edit: Fix incorrect size for format rect when it is smaller than text. user32/tests: Add test for edit control format rect size. https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
From: Jacob Czekalla <jczekalla(a)codeweavers.com> --- dlls/user32/tests/edit.c | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index bbde9b16616..663fe2ae31f 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -3426,6 +3426,55 @@ static void test_dbcs_WM_CHAR(void) } } +static void test_format_rect(void) +{ + HWND edit; + RECT rect, old_rect; + + static const struct + { + int style; + int style_ex; + RECT input; + int expected_equal; + } + tests[] = + { + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 10, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 250}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 250, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 1000}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 1000}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 0, 0}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 10, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 250}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 250, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 1000}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 1000}, 0} + }; + + 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)&old_rect); + 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)&rect); + + if (tests[i].expected_equal) + todo_wine ok(EqualRect(&old_rect, &rect), "Expected format rectangle to be equal to client rectangle.\n"); + else + todo_wine ok((rect.right - rect.left) > (old_rect.right - old_rect.left), "Expected format rect to be larger than client rectangle.\n"); + DestroyWindow(edit); + } +} + START_TEST(edit) { BOOL b; @@ -3464,6 +3513,7 @@ START_TEST(edit) test_EM_GETLINE(); test_wordbreak_proc(); test_dbcs_WM_CHAR(); + test_format_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 | 43 ++++++++++++++++++++++++++++++++-------- dlls/user32/tests/edit.c | 4 ++-- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/dlls/user32/edit.c b/dlls/user32/edit.c index 39f429813d6..0c7cb2947e2 100644 --- a/dlls/user32/edit.c +++ b/dlls/user32/edit.c @@ -2297,9 +2297,12 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) /* Windows doesn't care to fix text placement for SL controls */ es->format_rect.bottom = es->format_rect.top + es->line_height; - /* Always stay within the client area */ - GetClientRect(es->hwndSelf, &ClientRect); - es->format_rect.bottom = min(es->format_rect.bottom, ClientRect.bottom); + if (!(es->style & ES_MULTILINE)) + { + /* Always stay within the client area */ + GetClientRect(es->hwndSelf, &ClientRect); + es->format_rect.bottom = min(es->format_rect.bottom, ClientRect.bottom); + } if ((es->style & ES_MULTILINE) && !(es->style & ES_AUTOHSCROLL)) EDIT_BuildLineDefs_ML(es, 0, get_text_length(es), 0, NULL); @@ -2307,6 +2310,18 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) EDIT_SetCaretPos(es, es->selection_end, es->flags & EF_AFTER_WRAP); } +static int EDIT_is_valid_format_rect(const EDITSTATE *es, const RECT *rc) +{ + int text_height = es->line_height * es->line_count; + + if (IsRectEmpty(rc)) + return 0; + if (text_height > (rc->bottom - rc->top)) + return 0; + if (es->text_width > (rc->right - rc->left)) + return 0; + return 1; +} /********************************************************************* * @@ -2319,12 +2334,24 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) static void EDIT_SetRectNP(EDITSTATE *es, const RECT *rc) { LONG_PTR ExStyle; - INT bw, bh; + INT bw, bh, is_large = 0; + RECT edit_rect; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - - CopyRect(&es->format_rect, rc); - - if (ExStyle & WS_EX_CLIENTEDGE) { + + if (EDIT_is_valid_format_rect(es, rc)) + { + CopyRect(&es->format_rect, rc); + GetClientRect(es->hwndSelf, &edit_rect); + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) + is_large = 1; + } + else + { + GetClientRect(es->hwndSelf, &es->format_rect); + is_large = 0; + } + + if (ExStyle & WS_EX_CLIENTEDGE && !is_large) { es->format_rect.left++; es->format_rect.right--; diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index 663fe2ae31f..993b8a55297 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -3468,9 +3468,9 @@ static void test_format_rect(void) SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&rect); if (tests[i].expected_equal) - todo_wine ok(EqualRect(&old_rect, &rect), "Expected format rectangle to be equal to client rectangle.\n"); + ok(EqualRect(&old_rect, &rect), "Expected format rectangle to be equal to client rectangle.\n"); else - todo_wine ok((rect.right - rect.left) > (old_rect.right - old_rect.left), "Expected format rect to be larger than client rectangle.\n"); + ok((rect.right - rect.left) > (old_rect.right - old_rect.left), "Expected format rect to be larger than client rectangle.\n"); DestroyWindow(edit); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
From: Jacob Czekalla <jczekalla(a)codeweavers.com> Wine-Debug: https://bugs.winehq.org/show_bug.cgi?id=56873 --- dlls/comctl32/tests/edit.c | 50 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index bce0215ef19..c34234e647f 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -3791,6 +3791,55 @@ static void test_ime(void) DestroyWindow(hwnd); } +static void test_format_rect(void) +{ + HWND edit; + RECT rect, old_rect; + + static const struct + { + int style; + int style_ex; + RECT input; + int expected_equal; + } + tests[] = + { + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 10, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 250}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 250, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 1000}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 1000}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 0, 0}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 10, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 250}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 250, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 1000}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 10}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 1000}, 0} + }; + + 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)&old_rect); + 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)&rect); + + if (tests[i].expected_equal) + todo_wine ok(EqualRect(&old_rect, &rect), "Expected format rectangle to be equal to client rectangle.\n"); + else + todo_wine ok((rect.right - rect.left) > (old_rect.right - old_rect.left), "Expected format rect to be larger than client rectangle.\n"); + DestroyWindow(edit); + } +} + START_TEST(edit) { ULONG_PTR ctx_cookie; @@ -3838,6 +3887,7 @@ START_TEST(edit) test_change_focus(); test_cue_banner(); test_ime(); + test_format_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 | 39 ++++++++++++++++++++++++++++++++------ dlls/comctl32/tests/edit.c | 4 ++-- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c index 4b950412617..8505735d1ef 100644 --- a/dlls/comctl32/edit.c +++ b/dlls/comctl32/edit.c @@ -2222,9 +2222,12 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) /* Windows doesn't care to fix text placement for SL controls */ es->format_rect.bottom = es->format_rect.top + es->line_height; - /* Always stay within the client area */ - GetClientRect(es->hwndSelf, &ClientRect); - es->format_rect.bottom = min(es->format_rect.bottom, ClientRect.bottom); + if (!(es->style & ES_MULTILINE)) + { + /* Always stay within the client area */ + GetClientRect(es->hwndSelf, &ClientRect); + es->format_rect.bottom = min(es->format_rect.bottom, ClientRect.bottom); + } if ((es->style & ES_MULTILINE) && !(es->style & ES_AUTOHSCROLL)) EDIT_BuildLineDefs_ML(es, 0, get_text_length(es), 0, NULL); @@ -2232,6 +2235,18 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) EDIT_SetCaretPos(es, es->selection_end, es->flags & EF_AFTER_WRAP); } +static int EDIT_is_valid_format_rect(const EDITSTATE *es, const RECT *rc) +{ + int text_height = es->line_height * es->line_count; + + if (IsRectEmpty(rc)) + return 0; + if (text_height > (rc->bottom - rc->top)) + return 0; + if (es->text_width > (rc->right - rc->left)) + return 0; + return 1; +} /********************************************************************* * @@ -2244,12 +2259,24 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) static void EDIT_SetRectNP(EDITSTATE *es, const RECT *rc) { LONG_PTR ExStyle; - INT bw, bh; + INT bw, bh, is_large = 0; + RECT edit_rect; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - CopyRect(&es->format_rect, rc); + if (EDIT_is_valid_format_rect(es, rc)) + { + CopyRect(&es->format_rect, rc); + GetClientRect(es->hwndSelf, &edit_rect); + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) + is_large = 1; + } + else + { + GetClientRect(es->hwndSelf, &es->format_rect); + is_large = 0; + } - if (ExStyle & WS_EX_CLIENTEDGE) { + if (ExStyle & WS_EX_CLIENTEDGE && !is_large) { es->format_rect.left++; es->format_rect.right--; diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index c34234e647f..dd80dfcc013 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -3833,9 +3833,9 @@ static void test_format_rect(void) SendMessageA(edit, EM_GETRECT, 0, (LPARAM)&rect); if (tests[i].expected_equal) - todo_wine ok(EqualRect(&old_rect, &rect), "Expected format rectangle to be equal to client rectangle.\n"); + ok(EqualRect(&old_rect, &rect), "Expected format rectangle to be equal to client rectangle.\n"); else - todo_wine ok((rect.right - rect.left) > (old_rect.right - old_rect.left), "Expected format rect to be larger than client rectangle.\n"); + ok((rect.right - rect.left) > (old_rect.right - old_rect.left), "Expected format rect to be larger than client rectangle.\n"); DestroyWindow(edit); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
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=147957 Your paranoid android. === debian11 (32 bit report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 01090080, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000001EA00DA, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
+static void test_format_rect(void) +{ + HWND edit; + RECT rect, old_rect; + + static const struct + { + int style; + int style_ex; + RECT input; + int expected_equal; + } + tests[] = + { + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}, 1},
style in the tests are all the same so you can just use create_editcontrol(ES_MULTILINE | WS_VISIBLE, tests[i].style_ex); And let's use ex_style instead of style_ex. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79998
Zhiyi Zhang (@zhiyi) commented about dlls/user32/edit.c:
ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - - CopyRect(&es->format_rect, rc); - - if (ExStyle & WS_EX_CLIENTEDGE) { + + if (EDIT_is_valid_format_rect(es, rc)) + { + CopyRect(&es->format_rect, rc); + GetClientRect(es->hwndSelf, &edit_rect); + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) + is_large = 1; + } + else + { + GetClientRect(es->hwndSelf, &es->format_rect); This line still uses tabs. All changes should either keep using tabs or all the new changes should use spaces. Avoid mixing them in one commit.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_80000
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(const EDITSTATE *es, const RECT *rc) +{ + int text_height = es->line_height * es->line_count; + + if (IsRectEmpty(rc)) + return 0; + if (text_height > (rc->bottom - rc->top)) text_height is only used once so I think you can just use es->line_height * es->line_count.
Let's write it as "if (es->text_width > rc->right - rc->left || es->line_height * es->line_count > rc->bottom - rc->top)" -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79999
Zhiyi Zhang (@zhiyi) commented about dlls/user32/edit.c:
- - CopyRect(&es->format_rect, rc); - - if (ExStyle & WS_EX_CLIENTEDGE) { + + if (EDIT_is_valid_format_rect(es, rc)) + { + CopyRect(&es->format_rect, rc); + GetClientRect(es->hwndSelf, &edit_rect); + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) + is_large = 1; + } + else + { + GetClientRect(es->hwndSelf, &es->format_rect); + is_large = 0; You don't need to do this because is_large is initialized to 0 already. Or don't initialize is_large.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_80003
Zhiyi Zhang (@zhiyi) commented about dlls/user32/edit.c:
- if (ExStyle & WS_EX_CLIENTEDGE) { + + if (EDIT_is_valid_format_rect(es, rc)) + { + CopyRect(&es->format_rect, rc); + GetClientRect(es->hwndSelf, &edit_rect); + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) + is_large = 1; + } + else + { + GetClientRect(es->hwndSelf, &es->format_rect); + is_large = 0; + } + + if (ExStyle & WS_EX_CLIENTEDGE && !is_large) { Same here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_80001
Zhiyi Zhang (@zhiyi) commented about dlls/user32/edit.c:
LONG_PTR ExStyle; - INT bw, bh; + INT bw, bh, is_large = 0; + RECT edit_rect; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - - CopyRect(&es->format_rect, rc); - - if (ExStyle & WS_EX_CLIENTEDGE) { + + if (EDIT_is_valid_format_rect(es, rc)) + { + CopyRect(&es->format_rect, rc); + GetClientRect(es->hwndSelf, &edit_rect); + if ((rc->bottom - rc->top) > (edit_rect.bottom - edit_rect.top)) + is_large = 1; "too_large" might be more appropriate.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_80002
participants (4)
-
Jacob Czekalla -
Jacob Czekalla (＠JacobCzekalla)
-
Marvin -
Zhiyi Zhang (＠zhiyi)