Re[2]: [lmi] Wrong wxFlexGridSizer usage in about_dialog.cpp

[Vadim Zeitlin]

On Wed, 20 Jan 2010 01:48:18 +0000 Greg Chicares <address@hidden> wrote:

GC> In 'about_dialog.cpp', this seems to work correctly at the various
GC> resolutions I tested:
GC> 
GC>     html_window->GetInternalRepresentation()->Layout(450);
GC>     int const width  = html_window->GetInternalRepresentation()->GetWidth 
();
GC>     int const height = 
html_window->GetInternalRepresentation()->GetHeight();
GC>     html_window->SetMinSize(wxSize(width, height));
GC> 
GC> Is that reliable though?

 It should be, but it isn't :-(

GC> With the patch below, which gives Layout() a
GC> different 'width' argument, a vertical scrollbar appears

 This is the dreaded client size Escher-bug again. Unlike the better known
Heisen-bugs, this one can be reproduced perfectly every time but it's still
painful to deal with because it results from recursion which happens when
you set the client size of the window resulting in its relayout which
results in scrollbar [dis]appearing which results in the change of the
client size which ... I think you see what happens.

 In this particular case, the window is initially small (as no explicit
size is specified) and hence has the scrollbars. So when it's laid out, its
client width is not 600 but 583 for example (this is under Windows 7, I
think it should be 584 under XP, but what really matters is that it is <
600). Because of this the lines are slightly shorter and hence there is one
more of them. And because of this the window does need a vertical
scrollbar.

 The simplest and most straightforward (brutal?) way of fixing the bug is
this patch:

--- a/about_dialog.cpp
+++ b/about_dialog.cpp
@@ -66,7 +66,7 @@ int AboutDialog::ShowModal()
         ,wxID_ANY
         ,wxDefaultPosition
         ,wxDefaultSize
-        ,wxHW_SCROLLBAR_AUTO | wxHW_NO_SELECTION
+        ,wxHW_SCROLLBAR_NEVER | wxHW_NO_SELECTION
         );
     html_window->SetBorders(0);
     html_window->SetPage(license_notices_as_html());

This ensures that the bug never occurs because scrollbar is never shown. Of
course, it also ensures that the user won't be able to read the text at all
if for some reason the size of the HTML window is not big enough to show it
without the scrollbars.

 It would be better to fix this in wxHtmlWindow itself. I think (but would
appreciate any feedback from Vaclav about this) that its CreateLayout()
method might need to be modified to be smarter about the scrollbars. At the
very least we ought to make it check for reentrancy because currently the
window is relaid out half a dozen times even when it does work as expected
because calling SetScrollbars() already results in another call to
CreateLayout() and doing Layout() explicitly after it is unnecessary.

 Greg, please let me know if you think the above patch is an unacceptable
solution (AFAICS it should work fine with the current code) or if I should
try to fix the underlying problem in wxHtmlWindow itself.

 Thanks,
VZ