View Issue Details

IDProjectCategoryView StatusLast Update
0019278LazarusLCLpublic2017-04-30 16:59
ReporterMartin FriebeAssigned ToMichl 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version0.9.31 (SVN)Product Build 
Target Version1.8Fixed in Version1.9 (SVN) 
Summary0019278: W32: Pagecontrol, does show space for tabs, but no tabs in it (despite existing tabs)
DescriptionA bit tricky to reproduce:

Using a source-editor, with plenty of tabs open (so you get the <> scroll indicators on the right side of the pages-bar.
And enough tabs, so that if you close some, there will still be a need to scroll.

The best is to resize the window, so that only 2 tabs are showing. (and have 8 or more tabs open)

Scroll so that the last 2 tab are shown.

Close the last tab (middle click the tab.
- One tab will remain in the visible area
Close this new las tab.

No tabs are visible (only empty space) => besides there are plenty of tabs that could scroll in.

I had a particular case where the two scroll indicators disappeared too., so no way to get to the tabs then.
TagsNo tags attached.
Fixed in Revisionr54781
LazTarget-
WidgetsetWin32/Win64
Attached Files
  • example.zip (2,683 bytes)
  • win32.patch (4,373 bytes)
    Index: lcl/interfaces/win32/win32pagecontrol.inc
    ===================================================================
    --- lcl/interfaces/win32/win32pagecontrol.inc	(revision 54765)
    +++ lcl/interfaces/win32/win32pagecontrol.inc	(working copy)
    @@ -142,7 +142,7 @@
         RealIndex := TCustomTabControl(AWinControl.Parent).PageToTabIndex(PageIndex);
         if RealIndex <> -1 then
         begin
    -      Windows.SendMessage(PageControlHandle, TCM_DELETEITEM, Windows.WPARAM(RealIndex), 0);
    +      TWin32WSCustomTabControl.DeletePage(TCustomTabControl(AWinControl.Parent), RealIndex);
           AWinControl.Parent.InvalidateClientRectCache(False);
         end;
       end;
    @@ -149,7 +149,7 @@
       TWSWinControlClass(ClassParent).DestroyHandle(AWinControl);
     end;
     
    -class procedure TWin32WSCustomPage.ThemeChange(Wnd: HWnd);
    +class procedure TWin32WSCustomPage.ThemeChange(Wnd: HWND);
     var
       WindowInfo: PWin32WindowInfo;
     begin
    @@ -279,6 +279,64 @@
       Dec(ORect.Bottom, ARect.Bottom);
     end;
     
    +class procedure TWin32WSCustomTabControl.DeletePage(
    +  const ATabControl: TCustomTabControl; const AIndex: integer);
    +var
    +  ReorderingNeeded: Boolean;
    +  SelectedIndex, FirstShowedIndex, LastShowedIndex: Integer;
    +  Wnd: HWND;
    +
    +  function TabsReorderingNeeded: Boolean;
    +  var
    +    HitTestInfo: TC_HITTESTINFO;
    +    ARect: TRect;
    +    ItemCount: Integer;
    +  begin
    +    Windows.GetClientRect(Wnd, @ARect);
    +    Windows.SendMessage(Wnd, TCM_AdjustRect, 0, LPARAM(@ARect));
    +
    +    HitTestInfo.pt.x := ARect.Left;
    +    HitTestInfo.pt.y := ARect.Top div 2;
    +    FirstShowedIndex := Windows.SendMessage(Wnd, TCM_HITTEST, 0, LPARAM(@HitTestInfo));
    +
    +    if FirstShowedIndex = 0 then Exit(False); // all pages before are visible
    +
    +    LastShowedIndex := -1;
    +    HitTestInfo.pt.x := ARect.Right;
    +    while (LastShowedIndex = -1) and (HitTestInfo.pt.x > ARect.Left) do
    +    begin
    +      LastShowedIndex := Windows.SendMessage(Wnd, TCM_HITTEST, 0, LPARAM(@HitTestInfo));
    +      HitTestInfo.pt.x := HitTestInfo.pt.x - 5; // test all 5 pixel, if a tab is found
    +    end;
    +    ItemCount := Windows.SendMessage(Wnd, TCM_GETITEMCOUNT, 0, 0);
    +    if ItemCount - 1 > LastShowedIndex then Exit(False); // there are pages after the current visible tabs
    +
    +    SelectedIndex := Windows.SendMessage(Wnd, TCM_GETCURSEL, 0, 0);
    +    Result := (AIndex >= FirstShowedIndex) and (AIndex <= LastShowedIndex);
    +  end;
    +  procedure TabsReordering;
    +  begin
    +    if SelectedIndex >= AIndex then
    +      Dec(SelectedIndex);
    +    if LastShowedIndex >= AIndex then
    +      Dec(FirstShowedIndex);
    +    if FirstShowedIndex >= 0 then
    +      Windows.SendMessage(Wnd, TCM_SETCURSEL, Windows.WPARAM(FirstShowedIndex), 0);
    +    if SelectedIndex >= 0 then
    +      Windows.SendMessage(Wnd, TCM_SETCURSEL, Windows.WPARAM(SelectedIndex), 0);
    +  end;
    +
    +begin
    +  // to remove space at the end of pages we need a reordering, see mantis #19278
    +  // to get the reordering, the tab control is scrolled to the first position and back
    +  Wnd := ATabControl.Handle;
    +  ReorderingNeeded := TabsReorderingNeeded;
    +//  DebugLn('TWin32WSCustomPage.DeletePageEx First: ', dbgs(FirstShowedIndex), ' Last: ', dbgs(LastShowedIndex), ' Selected: ', dbgs(SelectedIndex), ' Delete: ', dbgs(Index));
    +  Windows.SendMessage(Wnd, TCM_DELETEITEM, Windows.WPARAM(AIndex), 0);
    +  if ReorderingNeeded then
    +    TabsReordering;
    +end;
    +
     class function TWin32WSCustomTabControl.CreateHandle(const AWinControl: TWinControl;
       const AParams: TCreateParams): HWND;
     const
    @@ -391,7 +449,7 @@
       if ATabControl is TTabControl then
         exit;
     
    -  Windows.SendMessage(ATabControl.Handle, TCM_DELETEITEM, Windows.WPARAM(AIndex), 0);
    +  DeletePage(ATabControl, AIndex);
       if LCLControlSizeNeedsUpdate(ATabControl, True) then
         AdjustSizeTabControlPages(ATabControl);
     end;
    Index: lcl/interfaces/win32/win32wscomctrls.pp
    ===================================================================
    --- lcl/interfaces/win32/win32wscomctrls.pp	(revision 54765)
    +++ lcl/interfaces/win32/win32wscomctrls.pp	(working copy)
    @@ -50,6 +50,8 @@
       { TWin32WSCustomTabControl }
     
       TWin32WSCustomTabControl = class(TWSCustomTabControl)
    +  public
    +    class procedure DeletePage(const ATabControl: TCustomTabControl; const AIndex: integer);
       published
         class function CreateHandle(const AWinControl: TWinControl;
               const AParams: TCreateParams): HWND; override;
    
    win32.patch (4,373 bytes)

Activities

Michl

2017-04-22 21:42

developer   ~0099789

Last edited: 2017-04-22 21:45

View 2 revisions

I looked at this issue for a while. A Tab Control is a bit poor managed from Windows side. There are no messages that can be used to reorder a Tab Control after deleting a tab. See https://msdn.microsoft.com/en-us/library/windows/desktop/ff486051(v=vs.85).aspx

I tested a lot of things with less benefit. The last thing, what I tried was to find the first and last real visible tabs before deleting the tab, set the Tab Control to the tab index 0 and set it back to first and last visible tabs.

It seems to work, also for the Lazarus Editor tabs but it is a ugly workaround, so I want to ask here what to do. Fix the issue with this not nice piece of code or let the behaviour as it is?

To test the issue, you can download the added example an press some time the button "Delete Last Page" and you will see the problem.

Also patch added, so you can see, what is needed to reorder the tabs (decrease the space at the end).

Thank you for feedback

José Mejuto

2017-04-23 01:48

reporter   ~0099793

I get the "desired" effect changing "delete las page" handler by:

procedure TForm1.Button2Click(Sender: TObject);
begin
  PageControl1.Pages[PageControl1.PageCount - 1].Free;
  TabControl1.Tabs.Delete(TabControl1.Tabs.Count - 1);
  RecreateWnd(TabControl1);
  RecreateWnd(PageControl1);
end;

I do not know if it is OK.

In the other hand when the pager dissapears the "tabs" are just there, but not visible, you can reach them with TAB key, resize the window and suddenly they appears (where resize forces pager to move some of them) but with a lot of drawing problems (some are drawn but others are not).

Michl

2017-04-23 15:40

developer   ~0099823

Thank you for your test, but no, that is not the way to go.

With RecreateWnd all Tab Control pages are deleted and created new. We just want to delete one of it. Also the current position isn't saved. If you select one of the first shown tabs and delete one, after deleting, the position isn't that one, you had before deleting. If you want to have the position, you must read it somehow. At the end, you come to the code like the added patch.

Btw. if we simply want to remove the space and not get the last position, this could be done very easy. Simply after deleting, move to tab 0 and back to the selected tab (on widgetset, not LCL). These would be two lines. But imho this isn't the preferred way, especially by closing a file in the Lazarus editor.

Martin Friebe

2017-04-23 18:01

manager   ~0099834

I think it would be wrong to *always* re-align.

If There are several tabs visible (on screen), and I close one of them, then no scroll needs to take place. (and it should not, for the user has not triggered scrolling).

But if the closed tab was the last visible, then this is different. Then we must make one other tab visible.

---------------
I tested, in the IDE, this issue does no longer happen (or at least I was not able to reproduce). The IDE sets another tab to be the new active tab, and that way this tab is scrolled in.

But in plain pagecontrols it still happens.
Even in the form designer. Add plenty of tabs, then start removing them.
Interestingly the object-inspector shows, that the tab-index is set to the new right-most tab, but that tab is not set to be visible (on screen).

And all this is, when every tab has its "visible" property = true. So none of them needs to be skipped.
Same at runtime.

---------------
Not sure why it does not work. There is code, that does set the new page index.

But I found something that may or may not be the problem.

The new Index is set in PageRemoved. Called by

procedure TCustomTabControl.RemovePage(Index: Integer);
    if HandleAllocated then
      AddRemovePageHandle(APage);
    PageRemoved(Index);
    TNBPages(FAccess).DeletePage(Index);

Note the order, first PageRemoved, then FAccess).DeletePage.

A few steps in this gets to

procedure TCustomTabControl.DoSendPageIndex;
  ShowCurrentPage;
  FPageIndexOnLastChange := FPageIndex;

This has the correct new Index.

ShowCurrentPage will set the visible property to true.
But it does more

procedure TCustomTabControl.ShowCurrentPage;
...
    CurPage.Visible := True;
...
  if (FPageIndexOnLastChange >= 0) and (FPageIndexOnLastChange < PageCount) and
     (FPageIndexOnLastChange <> FPageIndex) then
  begin
    Page[FPageIndexOnLastChange].Visible := False;
  end;

FPageIndexOnLastChange is the previous (just deleted) index. But it is still in PageCount, because TNBPages(FAccess).DeletePage has not yet been executed.

I am not sure that has any effect....

--------------------------------

Well and then I found something else.
Not in the code, but by experimentp

If I have 9 tabs / 3 visible at a time.
Initially I see tabs 7,8,9
I close tab 9 and 8.
Now closing tab 7 the following happens
- The page for tab 6 is shown
- No tabs are painted
 But if I click into the empty area, where tabs should be, then it switches to pages 1, 2 ,3. So there are invisible tabs there. And the wrong ones too.

Michl

2017-04-23 18:34

developer  

example.zip (2,683 bytes)

Michl

2017-04-23 18:41

developer   ~0099839

Last edited: 2017-04-23 18:47

View 5 revisions

@Martin: Did you try the patch? It works fine for me, for you too?

> But if I click into the empty area, where tabs should be, then it switches to
> pages 1, 2 ,3. So there are invisible tabs there. And the wrong ones too.

Yes, I adapted the example above. Now it shows the pages in the caption, when you are over a tab with the mouse. It seems to be a Windows bug, because tabs 0 upwards are localized now but not shown.


> I think it would be wrong to *always* re-align.
> If There are several tabs visible (on screen), and I close one of them, then
> no scroll needs to take place. (and it should not, for the user has not
> triggered scrolling).

This behaviour, I can also implement, the code will nearly the same. Only the function PagesReorderingNeeded should be adapted. Not such a big thing. I personally like no useless space ever.


Example:

[Page4][Page5][Page6] [</>]

By deleting Page5, I like to see (this is what the patch do).

[Page3][Page4][Page6] [</>]

You want to see this (or I'm wrong)?

[Page4][Page6] .space. [</>]

Michl

2017-04-30 16:54

developer  

win32.patch (4,373 bytes)
Index: lcl/interfaces/win32/win32pagecontrol.inc
===================================================================
--- lcl/interfaces/win32/win32pagecontrol.inc	(revision 54765)
+++ lcl/interfaces/win32/win32pagecontrol.inc	(working copy)
@@ -142,7 +142,7 @@
     RealIndex := TCustomTabControl(AWinControl.Parent).PageToTabIndex(PageIndex);
     if RealIndex <> -1 then
     begin
-      Windows.SendMessage(PageControlHandle, TCM_DELETEITEM, Windows.WPARAM(RealIndex), 0);
+      TWin32WSCustomTabControl.DeletePage(TCustomTabControl(AWinControl.Parent), RealIndex);
       AWinControl.Parent.InvalidateClientRectCache(False);
     end;
   end;
@@ -149,7 +149,7 @@
   TWSWinControlClass(ClassParent).DestroyHandle(AWinControl);
 end;
 
-class procedure TWin32WSCustomPage.ThemeChange(Wnd: HWnd);
+class procedure TWin32WSCustomPage.ThemeChange(Wnd: HWND);
 var
   WindowInfo: PWin32WindowInfo;
 begin
@@ -279,6 +279,64 @@
   Dec(ORect.Bottom, ARect.Bottom);
 end;
 
+class procedure TWin32WSCustomTabControl.DeletePage(
+  const ATabControl: TCustomTabControl; const AIndex: integer);
+var
+  ReorderingNeeded: Boolean;
+  SelectedIndex, FirstShowedIndex, LastShowedIndex: Integer;
+  Wnd: HWND;
+
+  function TabsReorderingNeeded: Boolean;
+  var
+    HitTestInfo: TC_HITTESTINFO;
+    ARect: TRect;
+    ItemCount: Integer;
+  begin
+    Windows.GetClientRect(Wnd, @ARect);
+    Windows.SendMessage(Wnd, TCM_AdjustRect, 0, LPARAM(@ARect));
+
+    HitTestInfo.pt.x := ARect.Left;
+    HitTestInfo.pt.y := ARect.Top div 2;
+    FirstShowedIndex := Windows.SendMessage(Wnd, TCM_HITTEST, 0, LPARAM(@HitTestInfo));
+
+    if FirstShowedIndex = 0 then Exit(False); // all pages before are visible
+
+    LastShowedIndex := -1;
+    HitTestInfo.pt.x := ARect.Right;
+    while (LastShowedIndex = -1) and (HitTestInfo.pt.x > ARect.Left) do
+    begin
+      LastShowedIndex := Windows.SendMessage(Wnd, TCM_HITTEST, 0, LPARAM(@HitTestInfo));
+      HitTestInfo.pt.x := HitTestInfo.pt.x - 5; // test all 5 pixel, if a tab is found
+    end;
+    ItemCount := Windows.SendMessage(Wnd, TCM_GETITEMCOUNT, 0, 0);
+    if ItemCount - 1 > LastShowedIndex then Exit(False); // there are pages after the current visible tabs
+
+    SelectedIndex := Windows.SendMessage(Wnd, TCM_GETCURSEL, 0, 0);
+    Result := (AIndex >= FirstShowedIndex) and (AIndex <= LastShowedIndex);
+  end;
+  procedure TabsReordering;
+  begin
+    if SelectedIndex >= AIndex then
+      Dec(SelectedIndex);
+    if LastShowedIndex >= AIndex then
+      Dec(FirstShowedIndex);
+    if FirstShowedIndex >= 0 then
+      Windows.SendMessage(Wnd, TCM_SETCURSEL, Windows.WPARAM(FirstShowedIndex), 0);
+    if SelectedIndex >= 0 then
+      Windows.SendMessage(Wnd, TCM_SETCURSEL, Windows.WPARAM(SelectedIndex), 0);
+  end;
+
+begin
+  // to remove space at the end of pages we need a reordering, see mantis #19278
+  // to get the reordering, the tab control is scrolled to the first position and back
+  Wnd := ATabControl.Handle;
+  ReorderingNeeded := TabsReorderingNeeded;
+//  DebugLn('TWin32WSCustomPage.DeletePageEx First: ', dbgs(FirstShowedIndex), ' Last: ', dbgs(LastShowedIndex), ' Selected: ', dbgs(SelectedIndex), ' Delete: ', dbgs(Index));
+  Windows.SendMessage(Wnd, TCM_DELETEITEM, Windows.WPARAM(AIndex), 0);
+  if ReorderingNeeded then
+    TabsReordering;
+end;
+
 class function TWin32WSCustomTabControl.CreateHandle(const AWinControl: TWinControl;
   const AParams: TCreateParams): HWND;
 const
@@ -391,7 +449,7 @@
   if ATabControl is TTabControl then
     exit;
 
-  Windows.SendMessage(ATabControl.Handle, TCM_DELETEITEM, Windows.WPARAM(AIndex), 0);
+  DeletePage(ATabControl, AIndex);
   if LCLControlSizeNeedsUpdate(ATabControl, True) then
     AdjustSizeTabControlPages(ATabControl);
 end;
Index: lcl/interfaces/win32/win32wscomctrls.pp
===================================================================
--- lcl/interfaces/win32/win32wscomctrls.pp	(revision 54765)
+++ lcl/interfaces/win32/win32wscomctrls.pp	(working copy)
@@ -50,6 +50,8 @@
   { TWin32WSCustomTabControl }
 
   TWin32WSCustomTabControl = class(TWSCustomTabControl)
+  public
+    class procedure DeletePage(const ATabControl: TCustomTabControl; const AIndex: integer);
   published
     class function CreateHandle(const AWinControl: TWinControl;
           const AParams: TCreateParams): HWND; override;
win32.patch (4,373 bytes)

Michl

2017-04-30 16:59

developer   ~0100017

I've done it, as you want it is done. Now only tab - 1 is shown, when the last shown tab is deleted.

Tested on Windows XP, Windows 7, Windows 10. Fixed in Trunk revision 54781.

Please test and close if ok.

Issue History

Date Modified Username Field Change
2011-05-02 12:27 Martin Friebe New Issue
2011-05-02 12:27 Martin Friebe LazTarget => -
2011-05-02 12:27 Martin Friebe Widgetset => Win32/Win64
2011-05-02 12:28 Martin Friebe Description Updated
2011-10-05 15:50 Vincent Snijders LazTarget - => 1.0
2011-10-05 15:50 Vincent Snijders Status new => acknowledged
2011-10-05 15:50 Vincent Snijders Target Version => 1.0.0
2012-02-04 13:58 Zeljan Rikalo LazTarget 1.0 => 1.2
2012-03-13 07:50 Vincent Snijders Target Version 1.0.0 => 1.2.0
2014-01-14 15:10 Martin Friebe LazTarget 1.2 => 1.4
2014-01-14 15:12 Martin Friebe Target Version 1.2.0 => 1.4
2014-09-10 10:43 Juha Manninen LazTarget 1.4 => -
2014-09-10 10:43 Juha Manninen Target Version 1.4 =>
2017-04-22 21:26 Michl File Added: example.zip
2017-04-22 21:26 Michl File Added: win32.patch
2017-04-22 21:42 Michl Note Added: 0099789
2017-04-22 21:42 Michl Assigned To => Michl
2017-04-22 21:42 Michl Status acknowledged => assigned
2017-04-22 21:45 Michl Note Edited: 0099789 View Revisions
2017-04-23 01:48 José Mejuto Note Added: 0099793
2017-04-23 15:40 Michl Note Added: 0099823
2017-04-23 18:01 Martin Friebe Note Added: 0099834
2017-04-23 18:33 Michl File Deleted: example.zip
2017-04-23 18:34 Michl File Added: example.zip
2017-04-23 18:41 Michl Note Added: 0099839
2017-04-23 18:42 Michl Note Edited: 0099839 View Revisions
2017-04-23 18:43 Michl Note Edited: 0099839 View Revisions
2017-04-23 18:46 Michl Note Edited: 0099839 View Revisions
2017-04-23 18:47 Michl Note Edited: 0099839 View Revisions
2017-04-30 16:53 Michl File Deleted: win32.patch
2017-04-30 16:54 Michl File Added: win32.patch
2017-04-30 16:59 Michl Fixed in Revision => r54781
2017-04-30 16:59 Michl Note Added: 0100017
2017-04-30 16:59 Michl Status assigned => resolved
2017-04-30 16:59 Michl Fixed in Version => 1.9 (SVN)
2017-04-30 16:59 Michl Resolution open => fixed
2017-04-30 16:59 Michl Target Version => 1.8