Re: [patch] Fix setup.exe chooser page header column borkage.

[Christopher Faylor <cgf-use-the-mailinglist-please at cygwin dot com>]

On Mon, Jun 29, 2009 at 04:50:38AM +0100, Dave Korn wrote:
>
>  There's a loophole in PickView.cc's handling of the header column widths.
>
>  You can see this by starting it up and running as far as the package chooser
>screen.  When it maximizes itself, click the view mode cycle button once and
>minimize it.  The header column display gets munged.  Note that you may need
>to be running on a fairly large desktop size to see it happen; I use
>1600x1200, and the delta in the x width when maximised compared to the
>previous window width at original size is crucial here.
>
>  Another way to reproduce the bug is to run through to the chooser and
>immediately minimize it.  Drag setup.exe to the left-hand-side of your screen,
>then grab its right-hand window frame edge and drag to resize the window as
>wide as it will go.  Switch from category to Full view mode, grab the edge and
>drag it to shrink the window back down again; the width of the header bar will
>suddenly jump to less than the chooser window size.  Do this a couple of times
>and you can resize the header bar right down to zero, it's cumulative.
>
>  What happens is that whenever a WM_SIZE message is received, the handler in
>PickView::WindowProc adjusts the width of the right-most column by the delta
>of the window size, to keep the overall header width in sync and filling the
>top of new chooser size.  The problem is that when you cycle to a new view
>mode, that changes over to a whole new set of header items, and when you then
>restore the window to its original size, the delta x is subtracted from the
>new final header item rather than the one it was originally added to.  On a
>wide enough desktop, this can then go negative, which windows does not render
>gracefully!
>
>  Fixed by making sure to always adjust both final header item widths by the
>same amount.  I had to break out the code that sets the column header ordering
>in order to have a way to find the last_col value for both cat_headers and
>pkg_headers that would be robust against future possible re-orderings and/or
>having a different number of columns in each.
>
>  Tested by both max/minimisation and free resizing around a switch between
>the header categories.
>
>	* PickView.cc (PickView::set_header_column_order):  New function,
>	broken out from ...
>	(PickView::set_headers):  ... here.  Call it.
>	(PickView::WindowProc):  Use it here to find and adjust final
>	column for both sets of headers.
>
>	* PickView.h (PickView::set_header_column_order):  Add prototype.
>
>  Ok?

Yes, except for minor whitespace nits.

The various indentation styles in setup really are annoying so I'd
rather not add to them.

>Index: PickView.cc
>===================================================================
>RCS file: /cvs/cygwin-apps/setup/PickView.cc,v
>retrieving revision 2.34
>diff -p -u -r2.34 PickView.cc
>--- PickView.cc	26 Jun 2009 15:09:07 -0000	2.34
>+++ PickView.cc	29 Jun 2009 03:30:18 -0000
>@@ -86,15 +86,15 @@ DoInsertItem (HWND hwndHeader, int iInse
>   return index;
> }
> 
>-void
>-PickView::set_headers ()
>+int
>+PickView::set_header_column_order (views vm)
> {
>-  if (view_mode == views::Unknown)
>-    return;
>-  if (view_mode == views::PackageFull ||
>-      view_mode == views::Package ||
>-      view_mode == views::PackageKeeps ||
>-      view_mode == views::PackageSkips)
>+  if (vm == views::Unknown)
>+    return -1;
>+  else if (vm == views::PackageFull ||
>+      vm == views::Package ||
>+      vm == views::PackageKeeps ||
>+      vm == views::PackageSkips)

Could you make sure that this is properly indented?

Actually, unless it has changed recently, the || is supposed to be at
the beginning of the line per GNU standards but that's not a new problem
here.

>Index: PickView.h
>===================================================================
>RCS file: /cvs/cygwin-apps/setup/PickView.h,v
>retrieving revision 2.18
>diff -p -u -r2.18 PickView.h
>--- PickView.h	24 Apr 2009 21:04:13 -0000	2.18
>+++ PickView.h	29 Jun 2009 03:30:18 -0000
>@@ -148,7 +148,8 @@ private:
>   // Stuff needed to handle resizing
>   bool hasClientRect;
>   RECTWrapper lastClientRect;
>-  
>+

This seems to be removing a trailing space right?  If so, thanks.

cgf