View Issue Details

IDProjectCategoryView StatusLast Update
0015614LazarusLCLpublic2016-11-13 23:48
ReporterZeljan RikaloAssigned ToPaul Ishenin 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version0.9.29 (SVN)Product Build 
Target Version0.9.30Fixed in Version0.9.29 (SVN) 
Summary0015614: TPopupMenu.Close() need rework
DescriptionAlready posted on devel list, but no interest, so opened issue about problem.
I've run into problems (especially under qtlcl under macosx) with calling
TPopupMenu.Close directly from WS.
Problem is because FItems handles are destroyed in TPopupMenu.Close each time it is called so some ws like qt on mac cannot handle that without huge delay (I've managed that via single shot timer).
I've attached patch which:
1.Doesn't touch FItems handles in TPopupMenu.Close - think delphi does not touch handles here too.
2.Removes FItems handles before each popup (if exists) - delphi
works in this way too - it rebuild handles each time before popup.
3.Destroy FItems handles in TPopupMenu.Destroy - so no memleaks

I've tested it and it works ok here. So, patch is here for review.
With this patch it's safe to call TPopupMenu.Close from WS (or we should
rename it / or add IntfPopupMenuClose ....)

Things like postponing such things (gtk via gtk_idle_add()) are bad.I've already tried to postpone it under qtlcl , but under macosx it needs >= 200ms of delay because it'll crash with smaller delay.
I think that this patch is much cleaner implementation of TPopupMenu.Close(), and gtk postponing can be removed if this patch is applied.

TagsNo tags attached.
Fixed in Revision23585
LazTarget0.9.30
WidgetsetGTK, GTK 2, Win32/Win64, WinCE, Carbon, QT, fpGUI
Attached Files
  • popupmenupatch.diff (728 bytes)
    Index: popupmenu.inc
    ===================================================================
    --- popupmenu.inc	(revision 23570)
    +++ popupmenu.inc	(working copy)
    @@ -59,6 +59,8 @@
     
     destructor TPopupMenu.Destroy;
     begin
    +  if HandleAllocated then
    +    FItems.DestroyHandle;
       Close;
       inherited Destroy;
     end;
    @@ -74,6 +76,10 @@
     procedure TPopupMenu.PopUp(X,Y : Integer);
     begin
       if ActivePopupMenu <> nil then ActivePopupMenu.Close;
    +
    +  if HandleAllocated then
    +    FItems.DestroyHandle;
    +
       FPopupPoint := Point(X, Y);
       ReleaseCapture;
       DoPopup(Self);
    @@ -87,8 +93,6 @@
     
     procedure TPopupMenu.Close;
     begin
    -  if HandleAllocated then
    -    FItems.DestroyHandle;
       if ActivePopupMenu = Self then
       begin
         DoClose;
    
    popupmenupatch.diff (728 bytes)

Relationships

related to 0009436 closedZeljan Rikalo Patches TPopupMenu.OnClose event does not work properly under gtk 
related to 0015595 closedDmitry Boyarintsev Lazarus IDE crashes when attempting to view lfm as source 
related to 0014144 closedPaul Ishenin Lazarus TPopupMenu inserted in TMainMenu works only when not popped-up standalone. 
related to 0025415 assignedMichl Lazarus LCL: TPopupMenu inserted in TmainMenu disappear after being shown by another control 

Activities

2010-01-28 19:00

 

popupmenupatch.diff (728 bytes)
Index: popupmenu.inc
===================================================================
--- popupmenu.inc	(revision 23570)
+++ popupmenu.inc	(working copy)
@@ -59,6 +59,8 @@
 
 destructor TPopupMenu.Destroy;
 begin
+  if HandleAllocated then
+    FItems.DestroyHandle;
   Close;
   inherited Destroy;
 end;
@@ -74,6 +76,10 @@
 procedure TPopupMenu.PopUp(X,Y : Integer);
 begin
   if ActivePopupMenu <> nil then ActivePopupMenu.Close;
+
+  if HandleAllocated then
+    FItems.DestroyHandle;
+
   FPopupPoint := Point(X, Y);
   ReleaseCapture;
   DoPopup(Self);
@@ -87,8 +93,6 @@
 
 procedure TPopupMenu.Close;
 begin
-  if HandleAllocated then
-    FItems.DestroyHandle;
   if ActivePopupMenu = Self then
   begin
     DoClose;
popupmenupatch.diff (728 bytes)

Paul Ishenin

2010-01-29 04:21

manager   ~0033992

DestroyHandle in the Close method was added by me as a part of 0009436

Paul Ishenin

2010-01-29 04:56

manager   ~0033993

Applied with modifications. Please test and close if ok.

Zeljan Rikalo

2010-01-29 08:41

developer   ~0034001

works fine. tnx.

Issue History

Date Modified Username Field Change
2010-01-28 19:00 Zeljan Rikalo New Issue
2010-01-28 19:00 Zeljan Rikalo File Added: popupmenupatch.diff
2010-01-28 19:00 Zeljan Rikalo LazTarget => -
2010-01-28 19:00 Zeljan Rikalo Widgetset => GTK, GTK 2, Win32/Win64, WinCE, Carbon, QT, fpGUI
2010-01-29 04:20 Paul Ishenin Relationship added related to 0009436
2010-01-29 04:21 Paul Ishenin Note Added: 0033992
2010-01-29 04:56 Paul Ishenin Fixed in Revision => 23585
2010-01-29 04:56 Paul Ishenin LazTarget - => 0.9.30
2010-01-29 04:56 Paul Ishenin Status new => resolved
2010-01-29 04:56 Paul Ishenin Fixed in Version => 0.9.29 (SVN)
2010-01-29 04:56 Paul Ishenin Resolution open => fixed
2010-01-29 04:56 Paul Ishenin Assigned To => Paul Ishenin
2010-01-29 04:56 Paul Ishenin Note Added: 0033993
2010-01-29 04:56 Paul Ishenin Target Version => 0.9.30
2010-01-29 06:44 Dmitry Boyarintsev Relationship added related to 0015595
2010-01-29 08:41 Zeljan Rikalo Status resolved => closed
2010-01-29 08:41 Zeljan Rikalo Note Added: 0034001
2010-03-22 21:24 Zeljan Rikalo Relationship added related to 0014144
2016-11-13 23:48 Juha Manninen Relationship added related to 0025415