From f78b3d1fc09db60a1b06d7be92bc971f303d579b Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Sun, 9 Sep 2012 00:44:26 +0000
Subject: [PATCH] Don't try to determine the clicked item ourselves in wxMSW
 wxListBox.

This doesn't work when the listbox is scrolled as the result of a click to
make the selected item fully visible and results in the index of the item
being off by 1 in the generated event which is a pretty serious problem.

Fix it by simply retrieving the item from the listbox itself, without doing
any hit testing. This seems to give the correct result in all cases and also
makes the code much simpler as we don't have to use 2 different ways of
finding the item depending on whether it was selected using the keyboard or
the mouse and makes it unnecessary to keep track of how the selection was done
completely, i.e. reverts r64498 which is not needed any more.

Closes #14635.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@72444 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775
---
 include/wx/msw/listbox.h |  6 ------
 src/msw/listbox.cpp      | 45 ++++++++--------------------------------
 2 files changed, 9 insertions(+), 42 deletions(-)

diff --git a/include/wx/msw/listbox.h b/include/wx/msw/listbox.h
index 0d9982bab8..b687aac8d7 100644
--- a/include/wx/msw/listbox.h
+++ b/include/wx/msw/listbox.h
@@ -151,8 +151,6 @@ public:
 
     virtual void OnInternalIdle();
 
-    virtual WXLRESULT MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lParam);
-
 protected:
     virtual wxSize DoGetBestClientSize() const;
 
@@ -194,10 +192,6 @@ private:
     // i.e. if we need to call SetHorizontalExtent() from OnInternalIdle()
     bool m_updateHorizontalExtent;
 
-    // flag set to true when we get a keyboard event and reset to false when we
-    // get a mouse one: this is used to find the correct item for the selection
-    // event
-    bool m_selectedByKeyboard;
 
     DECLARE_DYNAMIC_CLASS_NO_COPY(wxListBox)
 };
diff --git a/src/msw/listbox.cpp b/src/msw/listbox.cpp
index c01c325a5c..9f9720f39a 100644
--- a/src/msw/listbox.cpp
+++ b/src/msw/listbox.cpp
@@ -84,7 +84,6 @@ void wxListBox::Init()
 {
     m_noItems = 0;
     m_updateHorizontalExtent = false;
-    m_selectedByKeyboard = false;
 }
 
 bool wxListBox::Create(wxWindow *parent,
@@ -582,28 +581,23 @@ wxSize wxListBox::DoGetBestClientSize() const
 bool wxListBox::MSWCommand(WXUINT param, WXWORD WXUNUSED(id))
 {
     wxEventType evtType;
-    int n = wxNOT_FOUND;
     if ( param == LBN_SELCHANGE )
     {
         if ( HasMultipleSelection() )
             return CalcAndSendEvent();
 
         evtType = wxEVT_COMMAND_LISTBOX_SELECTED;
-
-        if ( m_selectedByKeyboard )
-        {
-            // We shouldn't use the mouse position to find the item as mouse
-            // can be anywhere, ask the listbox itself. Notice that this can't
-            // be used when the item is selected using the mouse however as
-            // LB_GETCARETINDEX will always return a valid item, even if the
-            // mouse is clicked below all the items, which is why we find the
-            // item ourselves below in this case.
-            n = SendMessage(GetHwnd(), LB_GETCARETINDEX, 0, 0);
-        }
-        //else: n will be determined below from the mouse position
     }
     else if ( param == LBN_DBLCLK )
     {
+        // Clicking under the last item in the listbox generates double click
+        // event for the currently selected item which is rather surprising.
+        // Avoid the surprise by checking that we do have an item under mouse.
+        const DWORD pos = ::GetMessagePos();
+        const wxPoint pt(GET_X_LPARAM(pos), GET_Y_LPARAM(pos));
+        if ( HitTest(ScreenToClient(pt)) == wxNOT_FOUND )
+            return false;
+
         evtType = wxEVT_COMMAND_LISTBOX_DOUBLECLICKED;
     }
     else
@@ -612,14 +606,7 @@ bool wxListBox::MSWCommand(WXUINT param, WXWORD WXUNUSED(id))
         return false;
     }
 
-    // Find the item position if it was a mouse-generated selection event or a
-    // double click event (which is always generated using the mouse)
-    if ( n == wxNOT_FOUND )
-    {
-        const DWORD pos = ::GetMessagePos();
-        const wxPoint pt(GET_X_LPARAM(pos), GET_Y_LPARAM(pos));
-        n = HitTest(ScreenToClient(wxPoint(pt)));
-    }
+    const int n = ListBox_GetCurSel(GetHwnd());
 
     // We get events even when mouse is clicked outside of any valid item from
     // Windows, just ignore them.
@@ -636,20 +623,6 @@ bool wxListBox::MSWCommand(WXUINT param, WXWORD WXUNUSED(id))
     return SendEvent(evtType, n, true /* selection */);
 }
 
-WXLRESULT
-wxListBox::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lParam)
-{
-    // Remember whether there was a keyboard or mouse event before
-    // LBN_SELCHANGE: this allows us to correctly determine the item affected
-    // by it in MSWCommand() above in any case.
-    if ( WM_KEYFIRST <= nMsg && nMsg <= WM_KEYLAST )
-        m_selectedByKeyboard = true;
-    else if ( WM_MOUSEFIRST <= nMsg && nMsg <= WM_MOUSELAST )
-        m_selectedByKeyboard = false;
-
-    return wxListBoxBase::MSWWindowProc(nMsg, wParam, lParam);
-}
-
 // ----------------------------------------------------------------------------
 // owner-drawn list boxes support
 // ----------------------------------------------------------------------------
-- 
2.45.2