[PATCH v10 0/4] MR6304: comctl32/edit: Fix incorrect size for format rect.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56873 -- v10: comctl32/edit: Fix incorrect size for format rect when it is smaller than text. https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
From: Jacob Czekalla <jczekalla(a)codeweavers.com> --- dlls/user32/tests/edit.c | 53 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index bbde9b16616..99a0073358c 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -3426,6 +3426,58 @@ static void test_dbcs_WM_CHAR(void) } } +static void test_format_rect(void) +{ + HWND edit; + RECT rect, expected_rect; + int size_flag = -2; + + struct state_tests + { + int style; + int style_ex; + RECT input; + int expected; + } + tests[] = + { + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 250}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 250, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 1000}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 1000}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 0, 0}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 250}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 250, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 1000}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 1000}, 1} + }; + + 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)&expected_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 (EqualRect(&expected_rect, &rect)) + size_flag = 0; + if ((expected_rect.right - expected_rect.left) > (rect.right - rect.left)) + size_flag = -1; + if ((rect.right - rect.left) > (expected_rect.right - expected_rect.left)) + size_flag = 1; + todo_wine ok(size_flag == tests[i].expected, "expected %d, but got %d\n", tests[i].expected, size_flag); + DestroyWindow(edit); + } +} + START_TEST(edit) { BOOL b; @@ -3464,6 +3516,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 | 51 ++++++++++++++++++++++++++++++++++------ dlls/user32/tests/edit.c | 2 +- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/dlls/user32/edit.c b/dlls/user32/edit.c index 39f429813d6..c59783463cf 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,24 @@ 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->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 -1; + return 1; +} /********************************************************************* * @@ -2319,12 +2340,28 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) static void EDIT_SetRectNP(EDITSTATE *es, const RECT *rc) { LONG_PTR ExStyle; - INT bw, bh; + INT bw, bh, res; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); + + res = EDIT_is_valid_format_rect(es, rc); + if (res == 0) + { + GetClientRect(es->hwndSelf, &es->format_rect); + } + else if (res == -1) + { + CopyRect(&es->format_rect, rc); + es->format_rect.left += 1; + es->format_rect.top += 1; + es->format_rect.right -= 1; + es->format_rect.bottom += 1; + } + else + { + CopyRect(&es->format_rect, rc); + } - CopyRect(&es->format_rect, rc); - - if (ExStyle & WS_EX_CLIENTEDGE) { + if (ExStyle & WS_EX_CLIENTEDGE && res != -1) { es->format_rect.left++; es->format_rect.right--; diff --git a/dlls/user32/tests/edit.c b/dlls/user32/tests/edit.c index 99a0073358c..2870150cee1 100644 --- a/dlls/user32/tests/edit.c +++ b/dlls/user32/tests/edit.c @@ -3473,7 +3473,7 @@ static void test_format_rect(void) size_flag = -1; if ((rect.right - rect.left) > (expected_rect.right - expected_rect.left)) size_flag = 1; - todo_wine ok(size_flag == tests[i].expected, "expected %d, but got %d\n", tests[i].expected, size_flag); + ok(size_flag == tests[i].expected, "expected %d, but got %d\n", tests[i].expected, size_flag); 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 | 53 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index bce0215ef19..f430c33523e 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -3791,6 +3791,58 @@ static void test_ime(void) DestroyWindow(hwnd); } +static void test_format_rect(void) +{ + HWND edit; + RECT rect, expected_rect; + int size_flag = -2; + + struct state_tests + { + int style; + int style_ex; + RECT input; + int expected; + } + tests[] = + { + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 0, 0}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {0, 0, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 250}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 250, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 10, 1000}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 1000}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 0, 0}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 250}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 250, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 1000}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 1000}, 1} + }; + + 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)&expected_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 (EqualRect(&expected_rect, &rect)) + size_flag = 0; + if ((expected_rect.right - expected_rect.left) > (rect.right - rect.left)) + size_flag = -1; + if ((rect.right - rect.left) > (expected_rect.right - expected_rect.left)) + size_flag = 1; + todo_wine ok(size_flag == tests[i].expected, "expected %d, but got %d\n", tests[i].expected, size_flag); + DestroyWindow(edit); + } +} + START_TEST(edit) { ULONG_PTR ctx_cookie; @@ -3838,6 +3890,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 | 55 +++++++++++++++++++++++++++++++++----- dlls/comctl32/tests/edit.c | 2 +- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c index 4b950412617..cad70816ae7 100644 --- a/dlls/comctl32/edit.c +++ b/dlls/comctl32/edit.c @@ -126,6 +126,7 @@ typedef struct INT wheelDeltaRemainder; /* scroll wheel delta left over after scrolling whole lines */ WCHAR *cue_banner_text; BOOL cue_banner_draw_focused; + INT valid_format_rect; /* * only for multi line controls @@ -2200,7 +2201,8 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) INT fw, vlc, max_x_offset, max_y_offset; vlc = get_vertical_line_count(es); - es->format_rect.bottom = es->format_rect.top + vlc * es->line_height; + if (es->valid_format_rect != -1) + es->format_rect.bottom = es->format_rect.top + vlc * es->line_height; /* correct es->x_offset */ fw = es->format_rect.right - es->format_rect.left; @@ -2222,9 +2224,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 +2237,24 @@ 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->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 -1; + return 1; +} /********************************************************************* * @@ -2244,12 +2267,29 @@ static void EDIT_AdjustFormatRect(EDITSTATE *es) static void EDIT_SetRectNP(EDITSTATE *es, const RECT *rc) { LONG_PTR ExStyle; - INT bw, bh; + INT bw, bh, res; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); - CopyRect(&es->format_rect, rc); + res = EDIT_is_valid_format_rect(es, rc); + if (res == 0) + { + GetClientRect(es->hwndSelf, &es->format_rect); + } + else if (res == -1) + { + CopyRect(&es->format_rect, rc); + es->format_rect.left += 1; + es->format_rect.top += 1; + es->format_rect.right -= 1; + es->format_rect.bottom -= 1; + } + else + { + CopyRect(&es->format_rect, rc); + } + es->valid_format_rect = res; - if (ExStyle & WS_EX_CLIENTEDGE) { + if (ExStyle & WS_EX_CLIENTEDGE && res != -1) { es->format_rect.left++; es->format_rect.right--; @@ -4381,6 +4421,7 @@ static LRESULT EDIT_WM_Create(EDITSTATE *es, const WCHAR *name) RECT clientRect; TRACE("%s\n", debugstr_w(name)); + es->valid_format_rect = 1; /* * To initialize some final structure members, we call some helper diff --git a/dlls/comctl32/tests/edit.c b/dlls/comctl32/tests/edit.c index f430c33523e..57989107380 100644 --- a/dlls/comctl32/tests/edit.c +++ b/dlls/comctl32/tests/edit.c @@ -3838,7 +3838,7 @@ static void test_format_rect(void) size_flag = -1; if ((rect.right - rect.left) > (expected_rect.right - expected_rect.left)) size_flag = 1; - todo_wine ok(size_flag == tests[i].expected, "expected %d, but got %d\n", tests[i].expected, size_flag); + ok(size_flag == tests[i].expected, "expected %d, but got %d\n", tests[i].expected, size_flag); DestroyWindow(edit); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6304
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; This is a new function. Let's use spaces instead tabs.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79732
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
+ {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 250}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 250, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 1000}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 1000}, 1} + }; + + 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)&expected_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 (EqualRect(&expected_rect, &rect)) So tests[i].expected can only be 0 or 1, which means EqualRect(&expected_rect, &rect) or (rect.right - rect.left) > (expected_rect.right - expected_rect.left). Thus I would suggest renaming "expected" to "expected_equal" and make it look like the following
if (tests[i].expected_equal)
todo_wine ok(EqualRect(&expected_rect, &rect), ...);
else
todo_wine ok((rect.right - rect.left) > (expected_rect.right - expected_rect.left), ...);
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79733
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
} }
+static void test_format_rect(void) +{ + HWND edit; + RECT rect, expected_rect; + int size_flag = -2; + + struct state_tests
Let's add static const here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79734
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
} }
+static void test_format_rect(void) +{ + HWND edit; + RECT rect, expected_rect; + int size_flag = -2; + + struct state_tests
You can delete state_tests. The type is not used anywhere. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79735
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) Add const for EDITSTATE * as well.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79736
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->top >= rc->bottom) Can you use IsRectEmpty(&rc) instead?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79737
Zhiyi Zhang (@zhiyi) commented about dlls/user32/tests/edit.c:
+ {ES_MULTILINE | WS_VISIBLE, 0, {1, 1, 1000, 1000}, 1}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 0, 0}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {0, 0, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 250}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 250, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 10, 1000}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 10}, 0}, + {ES_MULTILINE | WS_VISIBLE, WS_EX_CLIENTEDGE, {1, 1, 1000, 1000}, 1} + }; + + 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)&expected_rect); Let's rename expected_rect to old_rect. It seems more appropriate this way because the new rectangle doesn't always equal to the old rectangle.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79738
Zhiyi Zhang (@zhiyi) commented about dlls/user32/edit.c:
- INT bw, bh; + INT bw, bh, res; ExStyle = GetWindowLongPtrW(es->hwndSelf, GWL_EXSTYLE); + + res = EDIT_is_valid_format_rect(es, rc); + if (res == 0) + { + GetClientRect(es->hwndSelf, &es->format_rect); + } + else if (res == -1) + { + CopyRect(&es->format_rect, rc); + es->format_rect.left += 1; + es->format_rect.top += 1; + es->format_rect.right -= 1; + es->format_rect.bottom += 1; This looks hacky. What do these ones mean? Please find a way to simplify it. It's also a bit weird that EDIT_is_valid_format_rect() returns three values.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6304#note_79739
participants (3)
-
Jacob Czekalla -
Jacob Czekalla (＠JacobCzekalla)
-
Zhiyi Zhang (＠zhiyi)