[PATCH v11 0/4] MR6304: comctl32/edit: Fix incorrect size for format rect.

List overview All Threads

newer

older

[PATCH v5 0/1] MR4617: msxml3:...

[PATCH v4 0/2] MR6317: comctl32:...

Jacob Czekalla (＠JacobCzekalla)

23 Aug 2024 23 Aug '24

3:49 p.m.

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

Show replies by date

Jacob Czekalla

23 Aug 23 Aug

3:49 p.m.

New subject: [PATCH v11 1/4] user32/tests: Add test for edit control format rect size.

From: Jacob Czekalla jczekalla@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

Jacob Czekalla

3:50 p.m.

New subject: [PATCH v11 2/4] user32/edit: Fix incorrect size for format rect when it is smaller than text.

From: Jacob Czekalla jczekalla@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

Jacob Czekalla

3:50 p.m.

New subject: [PATCH v11 3/4] comctl32/tests: Add test for edit control format rect size.

From: Jacob Czekalla jczekalla@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

Jacob Czekalla

3:50 p.m.

New subject: [PATCH v11 4/4] comctl32/edit: Fix incorrect size for format rect when it is smaller than text.

From: Jacob Czekalla jczekalla@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;

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); }

/********************************************************************* * @@ -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);

-- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6304

Marvin

4:16 p.m.

New subject: [PATCH v11 4/4] comctl32/edit: Fix incorrect size for format rect when it is smaller than text.

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)

26 Aug 26 Aug

1:13 p.m.

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)

1:13 p.m.

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)

1:13 p.m.

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)

1:13 p.m.

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)

1:13 p.m.

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

Zhiyi Zhang (＠zhiyi)

1:13 p.m.

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

164

Age (days ago)

167

Last active (days ago)

wine-gitlab@winehq.org

11 comments

4 participants

tags (0)

participants (4)

Jacob Czekalla
Jacob Czekalla (＠JacobCzekalla)
Marvin
Zhiyi Zhang (＠zhiyi)