EDIT_EM_SetSel: Old/new selection range ordering code would break when

dlls/user/edit.c

@@ -3192,7 +3192,7 @@ static void EDIT_EM_ReplaceSel(EDITSTATE *es, BOOL can_undo, LPCWSTR lpsz_replac
 	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, UINT start, UINT end, BOOL after_wrap)
 		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, UINT start, UINT end, BOOL after_wrap)
  *          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 *es, LPCWSTR text, BOOL unicode)
 	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 *es, INT action, INT pos)
  */
 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(EDITSTATE *es, HRGN hrgn, BOOL bErase)
  */
 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);
 }

dlls/user/tests/edit.c

 /* 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, unsigned Height)
                  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();
 }