[PATCH v4 0/3] MR6608: riched20: Only run editor_ensure_visible and ME_SendRequestResize when control is in-place active.
According to microsoft documentation, GetClientRect, ScrollWindowEx, SetScrollPos and SetScrollRange (among others) may only be called while the control is in-place active. This fixes a segmentation fault in Anytone CPS. -- v4: riched20/tests: Test that ScrollWindowEx and GetClientRect are only called when control is in-place active. riched20: Only call ME_SendRequestResize when control is in-place active. riched20: Exit from editor_ensure_visible when control is not in-place active. https://gitlab.winehq.org/wine/wine/-/merge_requests/6608
From: Charlotte Pabst <cpabst(a)codeweavers.com> According to microsoft documentation, ScrollWindowEx, SetScrollPos and SetScrollRange may only be called while the control is in-place active. This fixes a segmentation fault in an application. Signed-off-by: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/riched20/paint.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/riched20/paint.c b/dlls/riched20/paint.c index 8bc1b8bcd2f..54b7ffd82bc 100644 --- a/dlls/riched20/paint.c +++ b/dlls/riched20/paint.c @@ -1212,6 +1212,8 @@ void editor_ensure_visible( ME_TextEditor *editor, ME_Cursor *cursor ) ME_Paragraph *para = cursor->para; int x, y, yheight; + if (!editor->in_place_active) + return; if (editor->scrollbars & ES_AUTOHSCROLL) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6608
From: Charlotte Pabst <cpabst(a)codeweavers.com> Prevent GetClientRect from being called when not in-place active. Signed-off-by: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/riched20/paint.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dlls/riched20/paint.c b/dlls/riched20/paint.c index 54b7ffd82bc..0182e54bfc6 100644 --- a/dlls/riched20/paint.c +++ b/dlls/riched20/paint.c @@ -110,10 +110,12 @@ void editor_draw( ME_TextEditor *editor, HDC hDC, const RECT *update ) if (oldRgn) DeleteObject( oldRgn ); ME_DestroyContext( &c ); - if (editor->nTotalLength != editor->nLastTotalLength || editor->nTotalWidth != editor->nLastTotalWidth) - ME_SendRequestResize(editor, FALSE); - editor->nLastTotalLength = editor->nTotalLength; - editor->nLastTotalWidth = editor->nTotalWidth; + if (editor->in_place_active) { + if (editor->nTotalLength != editor->nLastTotalLength || editor->nTotalWidth != editor->nLastTotalWidth) + ME_SendRequestResize(editor, FALSE); + editor->nLastTotalLength = editor->nTotalLength; + editor->nLastTotalWidth = editor->nTotalWidth; + } } void ME_Repaint(ME_TextEditor *editor) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6608
From: Charlotte Pabst <cpabst(a)codeweavers.com> Signed-off-by: Charlotte Pabst <cpabst(a)codeweavers.com> --- dlls/riched20/tests/txtsrv.c | 60 ++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/dlls/riched20/tests/txtsrv.c b/dlls/riched20/tests/txtsrv.c index adb36642f4f..0b4ac6c2369 100644 --- a/dlls/riched20/tests/txtsrv.c +++ b/dlls/riched20/tests/txtsrv.c @@ -103,6 +103,8 @@ static ITextServicesVtbl itextServicesStdcallVtbl; /* ITextHost implementation for conformance testing. */ DEFINE_EXPECT(ITextHostImpl_TxViewChange); +DEFINE_EXPECT(ITextHostImpl_TxScrollWindowEx); +DEFINE_EXPECT(ITextHostImpl_TxGetClientRect); typedef struct ITextHostTestImpl { @@ -268,6 +270,7 @@ static void __thiscall ITextHostImpl_TxScrollWindowEx(ITextHost *iface, INT dx, ITextHostTestImpl *This = impl_from_ITextHost(iface); TRACECALL("Call to TxScrollWindowEx(%p, %d, %d, %p, %p, %p, %p, %d)\n", This, dx, dy, lprcScroll, lprcClip, hRgnUpdate, lprcUpdate, fuScroll); + INCREASE_CALL_COUNTER(ITextHostImpl_TxScrollWindowEx); } static void __thiscall ITextHostImpl_TxSetCapture(ITextHost *iface, BOOL fCapture) @@ -321,6 +324,7 @@ static HRESULT __thiscall ITextHostImpl_TxGetClientRect(ITextHost *iface, LPRECT { ITextHostTestImpl *This = impl_from_ITextHost(iface); TRACECALL("Call to TxGetClientRect(%p, prc=%p)\n", This, prc); + INCREASE_CALL_COUNTER(ITextHostImpl_TxGetClientRect); *prc = This->client_rect; return S_OK; } @@ -1396,6 +1400,60 @@ static void test_scrollcaret( void ) ITextHost_Release( host ); } +static void test_inplace_active( BOOL active ) +{ + ITextServices *txtserv; + ITextHost *host; + LRESULT result; + HRESULT hr; + + ITextHostTestImpl *host_impl; + if (!init_texthost(&txtserv, &host)) + return; + + host_impl = impl_from_ITextHost( host ); + host_impl->window = CreateWindowExA( 0, "static", NULL, WS_POPUP|WS_VISIBLE, 0, 0, 100, 100, 0, 0, 0, NULL ); + host_impl->client_rect = (RECT) { 0, 0, 150, 150 }; + host_impl->scrollbars = WS_VSCROLL | ES_AUTOVSCROLL; + host_impl->props = TXTBIT_MULTILINE | TXTBIT_RICHTEXT | TXTBIT_WORDWRAP; + ITextServices_OnTxPropertyBitsChange( txtserv, TXTBIT_SCROLLBARCHANGE | TXTBIT_MULTILINE | TXTBIT_RICHTEXT | TXTBIT_WORDWRAP, host_impl->props ); + + CLEAR_COUNTER(ITextHostImpl_TxScrollWindowEx); + CLEAR_COUNTER(ITextHostImpl_TxGetClientRect); + + if (active) { + hr = ITextServices_OnTxInPlaceActivate( txtserv, &host_impl->client_rect ); + ok( hr == S_OK, "got %08lx\n", hr ); + } + + hr = ITextServices_TxSendMessage(txtserv, EM_SETEVENTMASK, 0, ENM_REQUESTRESIZE, &result); + ok( hr == S_OK, "got %08lx\n", hr ); + + hr = ITextServices_TxDraw( txtserv, DVASPECT_CONTENT, 0, NULL, NULL, GetDC(host_impl->window), NULL, (RECTL *)&host_impl->client_rect, NULL, NULL, NULL, 0, TXTVIEW_INACTIVE ); + ok( hr == S_OK, "got %08lx\n", hr ); + + hr = ITextServices_TxSendMessage(txtserv, EM_SETSEL, 0, -1, &result); + ok( hr == S_OK, "got %08lx\n", hr ); + + hr = ITextServices_TxSendMessage(txtserv, EM_REPLACESEL, TRUE, (LPARAM) lorem, &result); + ok( hr == S_OK, "got %08lx\n", hr ); + + hr = ITextServices_TxDraw( txtserv, DVASPECT_CONTENT, 0, NULL, NULL, GetDC(host_impl->window), NULL, (RECTL *)&host_impl->client_rect, NULL, NULL, NULL, 0, TXTVIEW_INACTIVE ); + ok( hr == S_OK, "got %08lx\n", hr ); + + if (active) { + CHECK_CALLED(ITextHostImpl_TxScrollWindowEx); + CHECK_CALLED(ITextHostImpl_TxGetClientRect); + } else { + CHECK_NOT_CALLED(ITextHostImpl_TxScrollWindowEx); + CHECK_NOT_CALLED(ITextHostImpl_TxGetClientRect); + } + + DestroyWindow( host_impl->window ); + ITextServices_Release( txtserv ); + ITextHost_Release( host ); +} + START_TEST( txtsrv ) { ITextServices *txtserv; @@ -1431,6 +1489,8 @@ START_TEST( txtsrv ) test_notifications(); test_set_selection_message(); test_scrollcaret(); + test_inplace_active(TRUE); + test_inplace_active(FALSE); } if (wrapperCodeMem) VirtualFree(wrapperCodeMem, 0, MEM_RELEASE); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6608
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=148836 Your paranoid android. === debian11 (32 bit ja:JP report) === riched20: txtsrv.c:1445: Test failed: expected ITextHostImpl_TxScrollWindowEx === debian11 (32 bit zh:CN report) === riched20: txtsrv.c:1445: Test failed: expected ITextHostImpl_TxScrollWindowEx === debian11b (64 bit WoW report) === kernel32: comm.c:1574: Test failed: AbortWaitCts hComPortEvent failed comm.c:1586: Test failed: Unexpected time 1001, expected around 500
Looks ok. Could you re-order the commits so that the tests come first (with added `todo_wine`s on the tests that'll fail). Then follow with the implementation changes, removing `todo_wine`s as appropriate. That way it's clear that your changes do indeed fix the tests that you're claiming they do. Also, it's no long necessary to add `Signed-off-by` tags. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6608#note_84307
participants (4)
-
Charlotte Pabst -
Charlotte Pabst (＠CharlottePabst)
-
Huw Davies (＠huw)
-
Marvin