[PATCH 0/2] MR326: riched20: Emulate a Windows bug that ignores undo/redo failure.
Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/326
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/riched20/tests/editor.c | 50 ++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/dlls/riched20/tests/editor.c b/dlls/riched20/tests/editor.c index 03c07829ee4..79ee7007873 100644 --- a/dlls/riched20/tests/editor.c +++ b/dlls/riched20/tests/editor.c @@ -7079,6 +7079,55 @@ static void test_undo_coalescing(void) DestroyWindow(hwnd); } +static void test_undo_status_bug(void) +{ + HWND hwnd; + LRESULT lresult; + + hwnd = new_richedit(NULL); + + lresult = SendMessageA(hwnd, EM_CANUNDO, 0, 0); + ok(lresult == FALSE, "EM_CANUNDO returned %Id\n", lresult); + lresult = SendMessageA(hwnd, EM_UNDO, 0, 0); + ok(lresult == FALSE, "EM_UNDO returned %Id\n", lresult); + lresult = SendMessageA(hwnd, EM_CANREDO, 0, 0); + ok(lresult == FALSE, "EM_CANREDO returned %Id\n", lresult); + lresult = SendMessageA(hwnd, EM_REDO, 0, 0); + ok(lresult == FALSE, "EM_REDO returned %Id\n", lresult); + + SendMessageA(hwnd, EM_SETSEL, 0, -1); + SendMessageA(hwnd, EM_REPLACESEL, TRUE, (LPARAM)"lorem"); + + lresult = SendMessageA(hwnd, EM_CANREDO, 0, 0); + ok(lresult == FALSE, "EM_CANREDO returned %Id\n", lresult); + lresult = SendMessageA(hwnd, EM_REDO, 0, 0); + ok(lresult == FALSE, "EM_REDO returned %Id\n", lresult); + + lresult = SendMessageA(hwnd, EM_CANUNDO, 0, 0); + ok(lresult == TRUE, "EM_CANUNDO returned %Id\n", lresult); + lresult = SendMessageA(hwnd, EM_UNDO, 0, 0); + ok(lresult == TRUE, "EM_UNDO returned %Id\n", lresult); + + lresult = SendMessageA(hwnd, EM_CANUNDO, 0, 0); + ok(lresult == FALSE, "EM_CANUNDO returned %Id\n", lresult); + lresult = SendMessageA(hwnd, EM_UNDO, 0, 0); + todo_wine + ok(lresult == TRUE, "EM_UNDO returned %Id\n", lresult); + + lresult = SendMessageA(hwnd, EM_CANREDO, 0, 0); + ok(lresult == TRUE, "EM_CANREDO returned %Id\n", lresult); + lresult = SendMessageA(hwnd, EM_REDO, 0, 0); + ok(lresult == TRUE, "EM_REDO returned %Id\n", lresult); + + lresult = SendMessageA(hwnd, EM_CANREDO, 0, 0); + ok(lresult == FALSE, "EM_CANREDO returned %Id\n", lresult); + lresult = SendMessageA(hwnd, EM_REDO, 0, 0); + todo_wine + ok(lresult == TRUE, "EM_REDO returned %Id\n", lresult); + + DestroyWindow(hwnd); +} + static LONG CALLBACK customWordBreakProc(WCHAR *text, int pos, int bytes, int code) { int length; @@ -9080,6 +9129,7 @@ START_TEST( editor ) test_EM_AUTOURLDETECT(); test_eventMask(); test_undo_coalescing(); + test_undo_status_bug(); test_word_movement(); test_EM_CHARFROMPOS(); test_SETPARAFORMAT(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/326
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=117710 Your paranoid android. === w7u_2qxl (32 bit report) === riched20: editor.c:6645: Test failed: wrong event mask (0x0) during WM_COMMAND
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/riched20/editor.c | 10 ++++++++-- dlls/riched20/editstr.h | 13 +++++++++++++ dlls/riched20/tests/editor.c | 2 -- dlls/riched20/undo.c | 2 ++ 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c index 3628e51ac9b..2eb8b7d2a11 100644 --- a/dlls/riched20/editor.c +++ b/dlls/riched20/editor.c @@ -2938,6 +2938,8 @@ ME_TextEditor *ME_MakeEditor(ITextHost *texthost, BOOL bEmulateVersion10) ed->bEmulateVersion10 = bEmulateVersion10; ed->in_place_active = FALSE; + ed->last_undo_status = FALSE; + ed->last_redo_status = FALSE; ed->total_rows = 0; ITextHost_TxGetPropertyBits( ed->texthost, TXTBIT_RICHTEXT | TXTBIT_MULTILINE | TXTBIT_READONLY | TXTBIT_USEPASSWORD | TXTBIT_HIDESELECTION | TXTBIT_SAVESELECTION | @@ -3313,9 +3315,13 @@ LRESULT editor_handle_message( ME_TextEditor *editor, UINT msg, WPARAM wParam, return !list_empty( &editor->redo_stack ); case WM_UNDO: /* FIXME: actually not the same */ case EM_UNDO: - return ME_Undo(editor); + if (editor->nUndoMode == umIgnore) return FALSE; + ME_Undo(editor); + return editor->last_undo_status; case EM_REDO: - return ME_Redo(editor); + if (editor->nUndoMode == umIgnore) return FALSE; + ME_Redo(editor); + return editor->last_redo_status; case EM_SETFONTSIZE: { CHARFORMAT2W cf; diff --git a/dlls/riched20/editstr.h b/dlls/riched20/editstr.h index 3b166234f23..2b6e00a861f 100644 --- a/dlls/riched20/editstr.h +++ b/dlls/riched20/editstr.h @@ -384,6 +384,19 @@ typedef struct tagME_TextEditor unsigned int bEmulateVersion10 : 1; unsigned int in_place_active : 1; unsigned int have_texthost2 : 1; + /* Windows Rich Edit has a bug where undos are counted regardless + * of whether they are actually peformed or not. It appears that + * each editor instance has an "undo successful" state variable + * that is set by an internal undo routine. However, failure to + * perform the operation (e.g. undo stack is empty) supposedly does + * not clear this state variable, which leads to reporting undo + * count that may be greater than the number of times it has + * actually happened. We emulate this bug by ignoring ME_Undo's + * return value and using the state variable directly. + */ + unsigned int last_undo_status : 1; + /* Ditto for redo. */ + unsigned int last_redo_status : 1; ME_TextBuffer *pBuffer; ME_Cursor *pCursors; DWORD props; diff --git a/dlls/riched20/tests/editor.c b/dlls/riched20/tests/editor.c index 79ee7007873..fc9178136e6 100644 --- a/dlls/riched20/tests/editor.c +++ b/dlls/riched20/tests/editor.c @@ -7111,7 +7111,6 @@ static void test_undo_status_bug(void) lresult = SendMessageA(hwnd, EM_CANUNDO, 0, 0); ok(lresult == FALSE, "EM_CANUNDO returned %Id\n", lresult); lresult = SendMessageA(hwnd, EM_UNDO, 0, 0); - todo_wine ok(lresult == TRUE, "EM_UNDO returned %Id\n", lresult); lresult = SendMessageA(hwnd, EM_CANREDO, 0, 0); @@ -7122,7 +7121,6 @@ static void test_undo_status_bug(void) lresult = SendMessageA(hwnd, EM_CANREDO, 0, 0); ok(lresult == FALSE, "EM_CANREDO returned %Id\n", lresult); lresult = SendMessageA(hwnd, EM_REDO, 0, 0); - todo_wine ok(lresult == TRUE, "EM_REDO returned %Id\n", lresult); DestroyWindow(hwnd); diff --git a/dlls/riched20/undo.c b/dlls/riched20/undo.c index d95ce121234..fdeb5e4d423 100644 --- a/dlls/riched20/undo.c +++ b/dlls/riched20/undo.c @@ -439,6 +439,7 @@ BOOL ME_Undo(ME_TextEditor *editor) table_move_from_row_start( editor ); add_undo( editor, undo_end_transaction ); + editor->last_undo_status = TRUE; editor->nUndoStackSize--; editor->nUndoMode = nMode; ME_UpdateRepaint(editor, FALSE); @@ -475,6 +476,7 @@ BOOL ME_Redo(ME_TextEditor *editor) } table_move_from_row_start( editor ); add_undo( editor, undo_end_transaction ); + editor->last_redo_status = TRUE; editor->nUndoMode = nMode; ME_UpdateRepaint(editor, FALSE); return TRUE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/326
Does anything actually rely on this bug? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/326#note_3197
On Mon Jul 4 14:35:03 2022 +0000, Huw Davies wrote:
Does anything actually rely on this bug? Not that I know of. This is mainly to make it easier for the implementation to satisfy the tests in https://gitlab.winehq.org/wine/wine/-/merge_requests/320, which is strict in the return value of `ITextDocument::Undo` and `ITextDocument::Redo` so that undo/redo can be tested more reliably.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/326#note_3235
On Mon Jul 4 14:35:03 2022 +0000, Jinoh Kang wrote:
Not that I know of. This is mainly to make it easier for the implementation to satisfy the tests in https://gitlab.winehq.org/wine/wine/-/merge_requests/320, which is strict in the return value of `ITextDocument::Undo` and `ITextDocument::Redo` so that undo/redo can be tested more reliably. In that case, please just relax the tests.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/326#note_3238
This merge request was closed by Jinoh Kang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/326
participants (4)
-
Huw Davies (＠huw) -
Jinoh Kang
-
Jinoh Kang (＠iamahuman)
-
Marvin