XE6 TListView column widths become zero if you read column.width
Asked Answered
B

0

2

There is a bug in TListView.

Reading a column's Width can cause the listview to try to get the column width from the underlying Windows LISTVIEW control directly - before the Win32 control's columns have been initialized.

Because the columns have not been initialized, the listview's LVM_GETCOLUMNWIDTH message fails, returning zero. The TListView takes this to mean that the width is zero, and makes all columns zero.

This bug was introduced sometime after Delphi 5.

Steps to reproduce

Add a report style listview with three columns to a form:

enter image description here

Add an OnResize event handler to the listview:

procedure TForm1.ListView1Resize(Sender: TObject);
begin
    {
       Any column you attempt to read the width of 
       will **cause** the width to become zero
    }
    ListView1.Columns[0].Width;
//  ListView1.Columns[1].Width;
    ListView1.Columns[2].Width;
end;

Run it:

enter image description here

The Bug

The TListColumn code tries to read the column width out of the Windows listview class directly, before the columns have even been added to the Windows listview control

Formatting their code for readability:

function TListColumn.GetWidth: TWidth;
var
   IsStreaming: Boolean;
   LOwner: TCustomListView;
begin
   LOwner := TListColumns(Collection).Owner;
   IsStreaming := [csReading, csWriting, csLoading] * LOwner.ComponentState <> [];

   if (
         (FWidth = 0) 
         and (LOwner.HandleAllocated or not IsStreaming)
      ) 
      or
      (
         (not AutoSize) 
         and LOwner.HandleAllocated 
         and (LOwner.ViewStyle = vsReport) 
         and (FWidth <> LVSCW_AUTOSIZE) 
         and (LOwner.ValidHeaderHandle)
      ) then
   begin
      FWidth := ListView_GetColumnWidth(LOwner.Handle, FOrderTag);
   end;

   Result := FWidth;
end; 

The problem happens while the form is being constructed during dfm deserialization:

ComCtrls.TListColumn.GetWidth: TWidth;
TForm1.ListView1Resize(Sender: TObject);
Windows.CreateWindowEx(...)
Controls.TWinControl.CreateWindowHandle(const Params: TCreateParams);
Controls.TWinControl.CreateWnd;
ComCtrls.TCustomListView.CreateWnd;

The issue is that TCustomListView.CreatWnd the columns are added sometime after the call to CreateWnd:

procedure TCustomListView.CreateWnd;
begin
   inherited CreateWnd; //triggers a call to OnResize, trying to read the column widths
   //...
   Columns.UpdateCols; //add the columns
   //...
end;

The code in TListColumn.GetWidth doesn't realize that the columns have not yet been initialized.

Why doesn't it fail in Delphi 5?

Delphi 5 uses similar TCustomListView construction:

procedure TCustomListView.CreateWnd;
begin
   inherited CreateWnd; //triggers a call to OnResize
   //...
   Columns.UpdateCols;
   //...
end;

Except Delphi 5 doesn't try to psyche itself out, and overthink things:

function TListColumn.GetWidth: TWidth;
begin
   if FWidth = 0 then
      FWidth := ListView_GetColumnWidth(TListColumns(Collection).Owner.Handle, Index);
   Result := FWidth;
end;

If we have a width, use it.

The question

Why was TListColumn.GetWidth changed? What bug were they trying to solve? I see they don't comment their code changes, so it's impossible to tell from the VCL source what the rationale was.

More importantly, how do i fix it? I cannot remove the code from OnResize, but i can create a TFixedListView custom control; except i would have to re-write everything from scratch so that it uses a TFixedListViewColumn class.

That's no good.

The most important question: How does Embarcadero fix it? What is code that should be in TListColumn.GetWidth to fix the bug? ComponentState is empty. It seems like they will have to introduce a new variable of:

FAreColumnsInitialized: Boolean;

Or they could put the code back to how it was.

What would you suggest they fix the code to?

Why does it work with themes disabled?

The bug only happens with Visual Styles enabled.

Windows has a WM_PARENTNOTIFY message that "notifies the parent of important events in the life of the control". In the case of the listview, this sends the handle of the header control that the listview uses internally. Delphi then saves this header hwnd:

procedure TCustomListView.WMParentNotify(var Message: TWMParentNotify);
begin
  with Message do
    if (Event = WM_CREATE) and (FHeaderHandle = 0) then
    begin
      FHeaderHandle := ChildWnd;

      //...
    end;
  inherited;
end;

With themes disabled, Windows does not send the WM_PARENTNOTIFY message until later in the construction cycle. This means that with themes disabled the TListColumn will fail to meet one of the criteria that allows it to talk to the listview:

   if (
         (FWidth = 0) 
         and (LOwner.HandleAllocated or not IsStreaming)
      ) 
      or
      (
         (not AutoSize) 
         and LOwner.HandleAllocated 
         and (LOwner.ViewStyle = vsReport) 
         and (FWidth <> LVSCW_AUTOSIZE) 
         and (LOwner.ValidHeaderHandle) //<--- invalid
      ) then
   begin
      FWidth := ListView_GetColumnWidth(LOwner.Handle, FOrderTag);
   end;

But when we use the new version of the Windows listview control, Windows happens to send the WM_PARENTNOTIFY message sooner in the construction:

   if (
         (FWidth = 0) 
         and (LOwner.HandleAllocated or not IsStreaming)
      ) 
      or
      (
         (not AutoSize) 
         and LOwner.HandleAllocated 
         and (LOwner.ViewStyle = vsReport) 
         and (FWidth <> LVSCW_AUTOSIZE) 
         and (LOwner.ValidHeaderHandle) //<--- Valid!
      ) then
   begin
      FWidth := ListView_GetColumnWidth(LOwner.Handle, FOrderTag);
   end;

Even through the header handle is valid, it does not mean that the columns have been added yet.

The Proposed Fix

It seems like the VCL fix is to use WM_PARENTNOTIY as the correct opportunity to add the columns to the listview:

procedure TCustomListView.WMParentNotify(var Message: TWMParentNotify);
begin
  with Message do
    if (Event = WM_CREATE) and (FHeaderHandle = 0) then
    begin
      FHeaderHandle := ChildWnd;
      UpdateCols; //20140822 Ian Boyd  Fixed QC123456 where the columns aren't usable in time
      //...
    end;
  inherited;
end;

Bonus Chatter

Looking at the Windows 2000 source code, the ListView has some comments that recognize some poor apps exist out there:

lvrept.c

BOOL_PTR NEAR ListView_CreateHeader(LV* plv)
{
   ...
   plv->hwndHdr = CreateWindowEx(0L, c_szHeaderClass, // WC_HEADER,
       NULL, dwStyle, 0, 0, 0, 0, plv->ci.hwnd, (HMENU)LVID_HEADER, GetWindowInstance(plv->ci.hwnd), NULL);

   if (plv->hwndHdr) 
   {
      NMLVHEADERCREATED nmhc;

      nmhc.hwndHdr = plv->hwndHdr;
      // some apps blow up if a notify is sent before the control is fully created.
      CCSendNotify(&plv->ci, LVN_HEADERCREATED, &nmhc.hdr);
      plv->hwndHdr = nmhc.hwndHdr;
   }
   ...
}
Bubbly answered 22/8, 2014 at 14:27 Comment(16)
Do you add a comment to your code every time you make a change? Or do use a version control system? Pointless asking why the change was made. What matters is how to fix it. My guess is that you might work around the issue by postponing the width reading code. Don't read width until some event that you can detect, after which there is no problem.Biodynamics
I do add a comment to my code time i make a change. I do use a version control system.Bubbly
That will, over time, make the code unreadable when the comments overwhelm the code. What's the point of having revision control if you don't use it to log change rationale? Or do you log that rationale in multiple places, thus duplicating the logging.Biodynamics
Bugs need to be reported to Quality Central (or its successor, Quality Portal) so Embarcadero is aware of them. There are other GetWidth() related bug reports: #22672, #26194, #30134Barite
@RemyLebeau Those bugs appear to be the exact same issue; reported 9 years ago and 8 years ago. Closed and closed and unfixed.Bubbly
If you feel they are the same issue, you can get them re-opened if you can provide test cases. Or just file a new bug report. Worse thing that happens is it gets marked as a duplicate.Barite
@RemyLebeau Are there VCL Fixpacks floating around for XE6? We've been debugging four VCL bugs today with our recent upgrade to XE6. I know Andreas had fixpacks up to XE. Is there a standard mechanism to easily patch the few hundred open bugs? Is there a team dedicated to patching these issues?Bubbly
@RemyLebeau The existing, closed, QC22672 is still reproducible. Use the exact code from the QC and it fails. I'm disheartened that it was closed at all. I convinced the company to fork over $3k, and we're getting bug after bug, after bug. And looking at the QC graveyard, i don't see any help coming. I don't know what to tell people.Bubbly
@IanBoyd: I re-opened 22672 and updated its IDE version to XE6. The latest IDE Fix Pack supports XE6, but I do not know if it includes VCL fixes.Barite
This has to be the longest most extensive question I've seen on SO. I didn't even know questions could be so long without hitting a limitation.Unscientific
@Ian QC is largely a total waste of time. It is massively dispiriting to submit bug report after bug report and have them ignored. Some do get fixed, but not nearly enough. My pet peeve is that Set80870CW is not threadsafe. And many related flaws in fp control word handling. But Emba won't listen to me it seems. In your position you need things fixed now so you'll need to do it yourself.Biodynamics
@JerryDodge That's nothing. You should see this question i asked a few years ago! It certainly is a balancing act. If you don't give enough information, people will answer in the simplest way (e.g. "did you try turning it off and on?"). So i try to pre-emptively thwart all the easy ways out by dealing with them myself (and, as a bonus, it shows i made research effort). But when a question becomes too complete, people's eyes glaze over and just back away slowly; like this comment.Bubbly
@DavidHeffernan "Dispiriting"; that's a good word for it. QC looks like where bugs go to die. And there are a lot of us who would be willing to fix things.Bubbly
@DavidHeffernan Co-worker was just debugging another bug in the VCL. "Kinda scares me that it's that buggy. It's like no one tested it. The whole idea was to upgrade to fix all the bugs."Bubbly
They claim that improve QA is one of their next goals. We shall see.Biodynamics
> "How does Embarcadero fix it? What is code that should be in TListColumn.GetWidth to fix the bug?" > The code needs to be elsewhere. Embarcadero should give up on setting values in getters.Kidwell

© 2022 - 2024 — McMap. All rights reserved.