Minor fixes after last commit:

1. don't pass -1 to base class DoSetSize(), this is documented to not work
   under Win2k
2. instead, don't do anything at all if neither size nor position changed
3. only call DoGetSize() once instead of doing it in both if branches
4. added comment about 30000 returned by CB_GETDROPPEDCONTROLRECT


git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@38308 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

src/msw/choice.cpp

@@ -236,7 +236,7 @@ int wxChoice::DoAppend(const wxString& item)
     return n;
 }
 
-int wxChoice::DoInsert(const wxString& item, int pos)
+int wxChoice::DoInsert(const wxString& item, unsigned int pos)
 {
     wxCHECK_MSG(!(GetWindowStyle() & wxCB_SORT), -1, wxT("can't insert into sorted list"));
     wxCHECK_MSG(IsValidInsert(pos), -1, wxT("invalid index"));
@@ -256,7 +256,7 @@ int wxChoice::DoInsert(const wxString& item, int pos)
     return n;
 }
 
-void wxChoice::Delete(int n)
+void wxChoice::Delete(unsigned int n)
 {
     wxCHECK_RET( IsValid(n), wxT("invalid item index in wxChoice::Delete") );
 
@@ -289,8 +289,8 @@ void wxChoice::Free()
 {
     if ( HasClientObjectData() )
     {
-        size_t count = GetCount();
+        unsigned int count = GetCount();
-        for ( size_t n = 0; n < count; n++ )
+        for ( unsigned int n = 0; n < count; n++ )
         {
             delete GetClientObject(n);
         }
@@ -326,9 +326,9 @@ void wxChoice::SetSelection(int n)
 // string list functions
 // ----------------------------------------------------------------------------
 
-size_t wxChoice::GetCount() const
+unsigned int wxChoice::GetCount() const
 {
-    return (size_t)SendMessage(GetHwnd(), CB_GETCOUNT, 0, 0);
+    return (unsigned int)SendMessage(GetHwnd(), CB_GETCOUNT, 0, 0);
 }
 
 int wxChoice::FindString(const wxString& s, bool bCase) const
@@ -336,11 +336,11 @@ int wxChoice::FindString(const wxString& s, bool bCase) const
 #if defined(__WATCOMC__) && defined(__WIN386__)
     // For some reason, Watcom in WIN386 mode crashes in the CB_FINDSTRINGEXACT message.
     // wxChoice::Do it the long way instead.
-    size_t count = GetCount();
+    unsigned int count = GetCount();
-    for ( size_t i = 0; i < count; i++ )
+    for ( unsigned int i = 0; i < count; i++ )
     {
         // as CB_FINDSTRINGEXACT is case insensitive, be case insensitive too
-        if ( GetString(i).IsSameAs(s, bCase) )
+        if (GetString(i).IsSameAs(s, bCase))
             return i;
     }
 
@@ -350,10 +350,10 @@ int wxChoice::FindString(const wxString& s, bool bCase) const
    //passed to SendMessage, so we have to do it ourselves in that case
    if ( s.empty() )
    {
-       size_t count = GetCount();
+       unsigned int count = GetCount();
-       for ( size_t i = 0; i < count; i++ )
+       for ( unsigned int i = 0; i < count; i++ )
        {
-         if ( GetString(i).empty() )
+         if (GetString(i).empty())
              return i;
        }
 
@@ -374,7 +374,7 @@ int wxChoice::FindString(const wxString& s, bool bCase) const
 #endif // Watcom/!Watcom
 }
 
-void wxChoice::SetString(int n, const wxString& s)
+void wxChoice::SetString(unsigned int n, const wxString& s)
 {
     wxCHECK_RET( IsValid(n), wxT("invalid item index in wxChoice::SetString") );
 
@@ -404,7 +404,7 @@ void wxChoice::SetString(int n, const wxString& s)
     InvalidateBestSize();
 }
 
-wxString wxChoice::GetString(int n) const
+wxString wxChoice::GetString(unsigned int n) const
 {
     int len = (int)::SendMessage(GetHwnd(), CB_GETLBTEXTLEN, n, 0);
 
@@ -430,7 +430,7 @@ wxString wxChoice::GetString(int n) const
 // client data
 // ----------------------------------------------------------------------------
 
-void wxChoice::DoSetItemClientData( int n, void* clientData )
+void wxChoice::DoSetItemClientData(unsigned int n, void* clientData)
 {
     if ( ::SendMessage(GetHwnd(), CB_SETITEMDATA,
                        n, (LPARAM)clientData) == CB_ERR )
@@ -439,7 +439,7 @@ void wxChoice::DoSetItemClientData( int n, void* clientData )
     }
 }
 
-void* wxChoice::DoGetItemClientData( int n ) const
+void* wxChoice::DoGetItemClientData(unsigned int n) const
 {
     LPARAM rc = SendMessage(GetHwnd(), CB_GETITEMDATA, n, 0);
     if ( rc == CB_ERR )
@@ -453,12 +453,12 @@ void* wxChoice::DoGetItemClientData( int n ) const
     return (void *)rc;
 }
 
-void wxChoice::DoSetItemClientObject( int n, wxClientData* clientData )
+void wxChoice::DoSetItemClientObject(unsigned int n, wxClientData* clientData)
 {
     DoSetItemClientData(n, clientData);
 }
 
-wxClientData* wxChoice::DoGetItemClientObject( int n ) const
+wxClientData* wxChoice::DoGetItemClientObject(unsigned int n) const
 {
     return (wxClientData *)DoGetItemClientData(n);
 }
@@ -506,36 +506,48 @@ void wxChoice::DoSetSize(int x, int y,
 {
     int heightOrig = height;
 
+    int widthCurrent, heightCurrent;
+    DoGetSize(&widthCurrent, &heightCurrent);
+
     // the height which we must pass to Windows should be the total height of
     // the control including the drop down list while the height given to us
     // is, of course, just the height of the permanently visible part of it
-    if ( height != wxDefaultCoord )
+    if ( height != wxDefaultCoord && height != heightCurrent )
     {
-        int w, h;
+        // don't make the drop down list too tall, arbitrarily limit it to 40
-        DoGetSize(&w, &h);
+        // items max and also don't leave it empty
+        size_t nItems = GetCount();
+        if ( !nItems )
+            nItems = 9;
+        else if ( nItems > 24 )
+            nItems = 24;
 
-        // Don't change the height if it's already this size
+        // add space for the drop down list
-        if (h == height)
+        const int hItem = SendMessage(GetHwnd(), CB_GETITEMHEIGHT, 0, 0);
-        {
+        height += hItem*(nItems + 1);
-            height = -1;
-        }
-        else
-        {
-            // don't make the drop down list too tall, arbitrarily limit it to 40
-            // items max and also don't leave it empty
-            size_t nItems = GetCount();
-            if ( !nItems )
-                nItems = 9;
-            else if ( nItems > 24 )
-                nItems = 24;
-            
-            // add space for the drop down list
-            const int hItem = SendMessage(GetHwnd(), CB_GETITEMHEIGHT, 0, 0);
-            height += hItem*(nItems + 1);
-        }
     }
-    else
+    else // keep the same height as now
     {
+        // normally wxWindow::DoSetSize() checks if we set the same size as the
+        // window already has and does nothing in this case, but for us the
+        // check fails as the size we pass to it includes the dropdown while
+        // the size returned by our GetSize() does not, so test if the size
+        // didn't really change ourselves here
+        if ( width == wxDefaultCoord || width == widthCurrent )
+        {
+            // size doesn't change, what about position?
+            int xCurrent, yCurrent;
+            DoGetPosition(&xCurrent, &yCurrent);
+            const bool defMeansUnchanged = !(sizeFlags & wxSIZE_ALLOW_MINUS_ONE);
+            if ( ((x == wxDefaultCoord && defMeansUnchanged) || x == xCurrent)
+                    &&
+                 ((y == wxDefaultCoord && defMeansUnchanged) || y == yCurrent) )
+            {
+                // nothing changes, nothing to do
+                return;
+            }
+        }
+
         // We cannot pass wxDefaultCoord as height to wxControl. wxControl uses
         // wxGetWindowRect() to determine the current height of the combobox,
         // and then again sets the combobox's height to that value. Unfortunately,
@@ -543,12 +555,15 @@ void wxChoice::DoSetSize(int x, int y,
         // on Win2K), so this would result in a combobox with dropdown height of
         // 1 pixel. We have to determine the default height ourselves and call
         // wxControl with that value instead.
-        int w, h;
+        //
+        // Also notice that sometimes CB_GETDROPPEDCONTROLRECT seems to return
+        // wildly incorrect values (~32000) which looks like a bug in it, just
+        // ignore them in this case
         RECT r;
-        DoGetSize(&w, &h);
+        if ( ::SendMessage(GetHwnd(), CB_GETDROPPEDCONTROLRECT, 0, (LPARAM) &r)
-        if (::SendMessage(GetHwnd(), CB_GETDROPPEDCONTROLRECT, 0, (LPARAM) &r) != 0 && r.bottom < 30000)
+                    && r.bottom < 30000 )
         {
-            height = h + r.bottom - r.top;
+            height = heightCurrent + r.bottom - r.top;
         }
     }
 
@@ -580,8 +595,8 @@ wxSize wxChoice::DoGetBestSize() const
 {
     // find the widest string
     int wChoice = 0;
-    const size_t nItems = GetCount();
+    const unsigned int nItems = GetCount();
-    for ( size_t i = 0; i < nItems; i++ )
+    for ( unsigned int i = 0; i < nItems; i++ )
     {
         int wLine;
         GetTextExtent(GetString(i), &wLine, NULL);