USER: edit.c bugfixes for 'update' (RESUBMIT)
C. Scott Ananian
cscott at cscott.net
Mon Mar 21 14:56:17 CST 2005
This is another patch (well, two separate patches originally) that seems
to have gotten lost. Originally sent last Thursday and Friday. This
patch fixes a bug in the 'sd' program (in the Wine app db):
command-completion incorrectly doubles the string, so that "tr{space}"
produces "tratra" instead of just adding the 'a' completion as expected.
ChangeLog:
- edit.c (EDIT_EM_SetSel): Old/new selection range ordering code would
break when
old_end < start < end < old_start
Rearrange the comparisons slightly to fix this; also document
the ordering relation this code is supposed to produce, and
verify optimality.
- edit.c (EDIT_UpdateText/EDIT_UpdateTextRegion): Reset EF_UPDATE flag
after we've sent the update.
- edit.c (EDIT_EM_ReplaceSel): send EN_CHANGE message, even if we've
already sent the EN_UPDATE message
- edit.c (EDIT_EM_SetText): add a test and call to ERR if we see the
preconditions for this bug occuring again (situation where SetSel
and ReplaceSel may not execute atomically).
- tests/edits.c: check that WM_SETTEXT doesn't generate spurious
intermediate update messages.
More information on the bug:
- WM_SETTEXT is implemented by selecting all text, and then replacing the
selection. Currently, the first 'select all' may generate an UPDATE
message (due to EF_UPDATE being stuck on), which can escape and be
handled by user code in unpleasant ways (for example, the user can reset the
selection during the handling of the update message). When SETTEXT
resumes, it does a 'replace all' -- but the selection may not still be
set correctly, and hilarity ensues.
--scott
Richard Tomlinson ODIBEX HOPEFUL SKIMMER ODEARL KGB SYNCARP corporate globalization
interception Mk 48 blowfish United Nations explosion Sugar Grove Qaddafi
( http://cscott.net/ )
-------------- next part --------------
Index: dlls/user/edit.c
===================================================================
RCS file: /home/wine/wine/dlls/user/edit.c,v
retrieving revision 1.20
diff -u -p -r1.20 edit.c
--- dlls/user/edit.c 25 Feb 2005 16:51:13 -0000 1.20
+++ dlls/user/edit.c 21 Mar 2005 20:34:17 -0000
@@ -3192,7 +3192,7 @@ static void EDIT_EM_ReplaceSel(EDITSTATE
EDIT_UpdateScrollInfo(es);
- if(es->flags & EF_UPDATE)
+ if(send_update || (es->flags & EF_UPDATE))
{
es->flags &= ~EF_UPDATE;
EDIT_NOTIFY_PARENT(es, EN_CHANGE, "EN_CHANGE");
@@ -3603,11 +3603,23 @@ static void EDIT_EM_SetSel(EDITSTATE *es
es->flags |= EF_AFTER_WRAP;
else
es->flags &= ~EF_AFTER_WRAP;
-/* This is a little bit more efficient than before, not sure if it can be improved. FIXME? */
- ORDER_UINT(start, end);
+ /* Compute the necessary invalidation region. */
+ /* Note that we don't need to invalidate regions which have
+ * "never" been selected, or those which are "still" selected.
+ * In fact, every time we hit a selection boundary, we can
+ * *toggle* whether we need to invalidate. Thus we can optimize by
+ * *sorting* the interval endpoints. Let's assume that we sort them
+ * in this order:
+ * start <= end <= old_start <= old_end
+ * Knuth 5.3.1 (p 183) asssures us that this can be done optimally
+ * in 5 comparisons; ie it's impossible to do better than the
+ * following: */
ORDER_UINT(end, old_end);
ORDER_UINT(start, old_start);
ORDER_UINT(old_start, old_end);
+ ORDER_UINT(start, end);
+ /* Note that at this point 'end' and 'old_start' are not in order, but
+ * start is definitely the min. and old_end is definitely the max. */
if (end != old_start)
{
/*
@@ -3616,6 +3628,7 @@ static void EDIT_EM_SetSel(EDITSTATE *es
* EDIT_InvalidateText(es, start, end);
* EDIT_InvalidateText(es, old_start, old_end);
* in place of the following if statement.
+ * (That would complete the optimal five-comparison four-element sort.)
*/
if (old_start > end )
{
@@ -4781,6 +4794,11 @@ static void EDIT_WM_SetText(EDITSTATE *e
text = textW;
}
+ if (es->flags & EF_UPDATE)
+ /* fixed this bug once; complain if we see it about to happen again. */
+ ERR("SetSel may generate UPDATE message whose handler may reset "
+ "selection.\n");
+
EDIT_EM_SetSel(es, 0, (UINT)-1, FALSE);
if (text)
{
@@ -5047,7 +5065,10 @@ static LRESULT EDIT_WM_VScroll(EDITSTATE
*/
static void EDIT_UpdateTextRegion(EDITSTATE *es, HRGN hrgn, BOOL bErase)
{
- if (es->flags & EF_UPDATE) EDIT_NOTIFY_PARENT(es, EN_UPDATE, "EN_UPDATE");
+ if (es->flags & EF_UPDATE) {
+ es->flags &= ~EF_UPDATE;
+ EDIT_NOTIFY_PARENT(es, EN_UPDATE, "EN_UPDATE");
+ }
InvalidateRgn(es->hwndSelf, hrgn, bErase);
}
@@ -5059,6 +5080,9 @@ static void EDIT_UpdateTextRegion(EDITST
*/
static void EDIT_UpdateText(EDITSTATE *es, LPRECT rc, BOOL bErase)
{
- if (es->flags & EF_UPDATE) EDIT_NOTIFY_PARENT(es, EN_UPDATE, "EN_UPDATE");
+ if (es->flags & EF_UPDATE) {
+ es->flags &= ~EF_UPDATE;
+ EDIT_NOTIFY_PARENT(es, EN_UPDATE, "EN_UPDATE");
+ }
InvalidateRect(es->hwndSelf, rc, bErase);
}
Index: dlls/user/tests/edit.c
===================================================================
RCS file: /home/wine/wine/dlls/user/tests/edit.c,v
retrieving revision 1.3
diff -u -p -r1.3 edit.c
--- dlls/user/tests/edit.c 14 Feb 2005 11:02:06 -0000 1.3
+++ dlls/user/tests/edit.c 21 Mar 2005 20:34:17 -0000
@@ -1,6 +1,7 @@
/* Unit test suite for edit control.
*
* Copyright 2004 Vitaliy Margolen
+ * Copyright 2005 C. Scott Ananian
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -19,6 +20,7 @@
#include <assert.h>
#include <windows.h>
+#include <windowsx.h>
#include <commctrl.h>
#include "wine/test.h"
@@ -27,6 +29,12 @@
#define ES_COMBO 0x200
#endif
+#define ID_EDITTEST2 99
+#define MAXLEN 200
+
+static char szEditTest2Name[] = "Edit Test 2 window class";
+static HINSTANCE hinst;
+static HWND hwndET2;
HWND create_editcontrol (DWORD style, DWORD exstyle)
{
@@ -79,7 +87,7 @@ static void set_client_height(HWND Wnd,
SWP_NOMOVE | SWP_NOZORDER);
}
-static void test_edit_control(void)
+static void test_edit_control_1(void)
{
HWND hwEdit;
MSG msMessage;
@@ -268,7 +276,95 @@ static void test_edit_control(void)
DestroyWindow(hwEdit);
}
+/* WM_SETTEXT is implemented by selecting all text, and then replacing the
+ * selection. This test checks that the first 'select all' doesn't generate
+ * an UPDATE message which can escape and (via a handler) change the
+ * selection, which would cause WM_SETTEXT to break. This old bug
+ * was fixed 18-Mar-2005; we check here to ensure it doesn't regress.
+ */
+static void test_edit_control_2(void)
+{
+ HWND hwndMain;
+ char szLocalString[MAXLEN];
+
+ /* Create main and edit windows. */
+ hwndMain = CreateWindow(szEditTest2Name, "ET2", WS_OVERLAPPEDWINDOW,
+ 0, 0, 200, 200, NULL, NULL, hinst, NULL);
+ assert(hwndMain);
+ if (winetest_interactive)
+ ShowWindow (hwndMain, SW_SHOW);
+
+ hwndET2 = CreateWindow("EDIT", NULL,
+ WS_CHILD|WS_BORDER|ES_LEFT|ES_AUTOHSCROLL,
+ 0, 0, 150, 50, // important this not be 0 size.
+ hwndMain, (HMENU) ID_EDITTEST2, hinst, NULL);
+ assert(hwndET2);
+ if (winetest_interactive)
+ ShowWindow (hwndET2, SW_SHOW);
+
+ trace("EDIT: SETTEXT atomicity\n");
+ /* Send messages to "type" in the word 'foo'. */
+ SendMessage(hwndET2, WM_CHAR, 'f', 1);
+ SendMessage(hwndET2, WM_CHAR, 'o', 1);
+ SendMessage(hwndET2, WM_CHAR, 'o', 1);
+ /* 'foo' should have been changed to 'bar' by the UPDATE handler. */
+ GetWindowText(hwndET2, szLocalString, MAXLEN);
+ ok(lstrcmp(szLocalString, "bar")==0,
+ "Wrong contents of edit: %s\n", szLocalString);
+
+ /* OK, done! */
+ DestroyWindow (hwndET2);
+ DestroyWindow (hwndMain);
+}
+
+static void ET2_check_change() {
+ char szLocalString[MAXLEN];
+ /* This EN_UPDATE handler changes any 'foo' to 'bar'. */
+ GetWindowText(hwndET2, szLocalString, MAXLEN);
+ if (lstrcmp(szLocalString, "foo")==0) {
+ lstrcpy(szLocalString, "bar");
+ SendMessage(hwndET2, WM_SETTEXT, 0, (LPARAM) szLocalString);
+ }
+ /* always leave the cursor at the end. */
+ SendMessage(hwndET2, EM_SETSEL, MAXLEN - 1, MAXLEN - 1);
+}
+static void ET2_OnCommand(HWND hwnd, int id, HWND hwndCtl, UINT codeNotify)
+{
+ if (id==ID_EDITTEST2 && codeNotify == EN_UPDATE)
+ ET2_check_change();
+}
+static LRESULT CALLBACK ET2_WndProc(HWND hwnd, UINT iMsg, WPARAM wParam, LPARAM lParam)
+{
+ switch (iMsg) {
+ HANDLE_MSG(hwnd, WM_COMMAND, ET2_OnCommand);
+ }
+ return DefWindowProc(hwnd, iMsg, wParam, lParam);
+}
+
+static BOOL RegisterWindowClasses (void)
+{
+ WNDCLASSA cls;
+ cls.style = 0;
+ cls.lpfnWndProc = ET2_WndProc;
+ cls.cbClsExtra = 0;
+ cls.cbWndExtra = 0;
+ cls.hInstance = hinst;
+ cls.hIcon = NULL;
+ cls.hCursor = LoadCursorA (NULL, IDC_ARROW);
+ cls.hbrBackground = (HBRUSH)(COLOR_WINDOW + 1);
+ cls.lpszMenuName = NULL;
+ cls.lpszClassName = szEditTest2Name;
+ if (!RegisterClassA (&cls)) return FALSE;
+
+ return TRUE;
+}
+
START_TEST(edit)
{
- test_edit_control();
+ hinst = GetModuleHandleA (NULL);
+ if (!RegisterWindowClasses())
+ assert(0);
+
+ test_edit_control_1();
+ test_edit_control_2();
}
More information about the wine-patches
mailing list