#15723 closed defect (fixed)

wxChoice may report its width as negative on wxMSW during deferred sizing

Reported by: ikamakj Owned by:
Priority: normal Milestone:
Component: wxMSW Version: dev-latest
Keywords: Cc:
Blocked By: Blocking:
Patch: no

Description

DoSetSize() may be called with width and height equal to wxDefaultCoord (-1), which occurs e.g. via wxWindow::Move(). This does not work for wxChoice on MSW if there is already a pending size change. Since the height argument in wxChoice::DoSetSize() is wxDefaultCoord, the initial lines of the function replace it by the current size, while the width argument remains -1. Since m_pendingSize is not wxDefaultSize, m_pendingSize is updated to (-1, height), which is definitely wrong.

If the size is then queried before the deferred sizing has occurred, m_pendingSize still has the wrong value, and wxWindowMSW::DoGetSize() returns -1 for the width.

Change History (5)

comment:1 Changed 12 months ago by vadz

  • Status changed from new to confirmed
  • Version changed from 2.9.5 to dev-latest

If I follow your explanation correctly, I think this patch should fix the problem:

  • src/msw/choice.cpp

    diff --git a/src/msw/choice.cpp b/src/msw/choice.cpp
    index 6699e1e..f74d647 100644
    a b void wxChoice::DoSetSize(int x, int y, 
    533533                         int width, int height, 
    534534                         int sizeFlags) 
    535535{ 
    536     const int heightBest = GetBestSize().y; 
     536    // The height of the control itself, i.e. of its visible part. 
     537    int heightVisible = height; 
    537538 
    538539    // we need the real height below so get the current one if it's not given 
    539540    if ( height == wxDefaultCoord ) 
    540541    { 
    541542        // height not specified, use the same as before 
    542         DoGetSize(NULL, &height); 
     543        DoGetSize(NULL, &heightVisible); 
    543544    } 
    544     else if ( height == heightBest ) 
     545    else if ( height == GetBestSize().y ) 
    545546    { 
    546547        // we don't need to manually manage our height, let the system use the 
    547548        // default one 
    void wxChoice::DoSetSize(int x, int y, 
    585586        // The extra item (" + 1") is required to prevent a vertical 
    586587        // scrollbar from appearing with comctl32.dll versions earlier 
    587588        // than 6.0 (such as found in Win2k). 
    588         heightWithItems = height + hItem*(nItems + 1); 
     589        heightWithItems = heightVisible + hItem*(nItems + 1); 
    589590    else 
    590591        heightWithItems = SetHeightSimpleComboBox(nItems); 
    591592 

Could you please test it? TIA!

comment:2 Changed 12 months ago by ikamakj

Sorry for the delay. Perhaps I'm not aware of the subtleties of the deferred sizing, but I didn't quite understand how this patch would fix the problem. After applying the patch, m_pendingSize gets the original values of width and height, that is, (-1, -1) in the situation I described. Can you really do this here?

comment:3 Changed 12 months ago by vadz

Perhaps I misunderstood the problem when I looked at this the first time, but I thought it was due to m_pendingSize having an invalid value. There is an invariant that it must be either (-1, -1) (default) or have both valid components and it was broken, so I fixed this.

What is exactly the problem remaining after the patch?

comment:4 Changed 10 months ago by vadz

I'm going to commit the fix above, please reopen with the instructions about what the problem is exactly and how can it be reproduced if it's still there.

comment:5 Changed 10 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [75654]) Ensure wxChoice reported width never becomes negative in wxMSW.

Don't set the pending size to (-1, valid-height) as it is always supposed to
be either fully valid or fully invalid (meaning there is no pending size
change).

Closes #15723.

Note: See TracTickets for help on using tickets.