All Projects FPC fpcprojects fpprofilerfpGUI Lazarus Packages PatchesLazarus CCR Mantis pas2js

View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0024476LazarusLCLpublic2013-05-25 20:532017-02-11 23:49
ReporterBenito van der Zander Assigned ToJuha Manninen  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version1.1 (SVN) 
Summary0024476: submenu on separator crashes
DescriptionIf you create a menuitem that is a separator and has a submenu, the program crashes, if the menu is opened.

Steps To ReproduceRight-click on the form:

object Form1: TForm1
  Left = 376
  Height = 240
  Top = 338
  Width = 320
  Caption = 'Form1'
  PopupMenu = PopupMenu1
  LCLVersion = '1.1'
  object PopupMenu1: TPopupMenu
    left = 143
    top = 61
    object MenuItem1: TMenuItem
      Caption = '-'
      object MenuItem2: TMenuItem
        Caption = 'New Item2'
      end
    end
  end
end
TagsNo tags attached.
Fixed in Revision
LazTarget-
WidgetsetGTK 2
Attached Files

Relationships

related to 0027905 closedZeljan Rikalo Lazarus GTK2 menu separator item (patch) 
related to 0029205 resolvedOndrej Pokorny Patches patch and files to implement a new IDE menueditor 
related to 0031138 resolvedJuha Manninen Lazarus TMainMenu Seperator causes exception 

Activities

Zeljan Rikalo

2013-05-27 16:21

developer   ~0067905

Will be hard to fix. It completely locks X11 when trying to run it via gdb.

Bart Broersma

2015-03-15 16:41

developer   ~0081993

Last edited: 2015-03-15 16:43

View 2 revisions

 Should the menu-designer forbid this?
In Delphi 7 I can create such a submenu, but it won't be shown (although it works through it's shotcut, why, o, why).

Bart Broersma

2015-03-17 19:49

developer   ~0082039

Nice, instant freeze of Linux when I run the program.


Disallowing adding submenusś works as a workaround, however when i tried to ifdef it for GTK2, it turned out that this was not defined (yet) at the time the menu was created.

procedure TMenuItem.Add(Item: TMenuItem);
begin
  {$ifdef lclgtk2}
  if Self.IsLine then
  begin
    debugln('GTK2 crashes if a MenuItem with Caption "-" has subitems');
    Exit;
  end;
  {$endif}
  Insert(GetCount, Item);
end;


Since QT and Win32 don crash, disabling it for all widgetsets seems a bit crude.

Zeljan Rikalo

2015-03-17 20:10

developer   ~0082040

@Bart, haven't I noticed that X11 will be locked ? ;)

Bart Broersma

2015-03-17 21:40

developer   ~0082044

@Zeljan: you said: when running via GDB. I just ran the executable from the console. :-(

Zeljan Rikalo

2015-03-18 08:41

developer   ~0082049

Tried only under gdb, so didn't want to kill X again, just to see if it locks without gdb :)

Bart Broersma

2015-03-18 11:26

developer   ~0082052

Q1:
Why isn't LCLGTK2 defined at the point of constructiong the menu?

Q2:
What is the use-case of a menu-separator having a submenu?
Is it intended behaviour, or does it just "happen to work" on Windows?
If it's just a by-product and not documented to work (as in official Delphi documentation) then the developer should not rely on this functionality being available, and the menu-designer should forbid it (and maybe TMenuIte.Add should too).

Zeljan Rikalo

2015-03-18 11:31

developer   ~0082053

If someone needs hidden menu like that, TAction is there I guess.
That should be forbidden (submenu of separator menu item)

Bart Broersma

2015-03-18 11:37

developer   ~0082054

This code on StackOverflow (http://stackoverflow.com/questions/7505581/how-to-add-menu-items-separators-which-work-as-expected-on-osx) which fixes a bug in FireMonkey also seems to assume that separator menu-items cannot have submenu's on OSX.

Bart Broersma

2015-03-18 11:43

developer   ~0082057

> That should be forbidden (submenu of separator menu item)

I guess first of all in Menu-designer.
Should we also forbid it in TmenuItem.Add?

Zeljan Rikalo

2015-03-18 12:55

developer   ~0082065

Yes, and raise exception.

Bart Broersma

2015-03-18 13:07

developer   ~0082068

Last edited: 2015-03-18 13:07

View 2 revisions

 The menu-designer is a beast.
I managed in disabling creating submenu-items for a separator, but I'm not happy with the way I did it.

Basically we want:
- disable the "Add submenu" entry in the popupmenu if popped-up on a separator
- disallow setting caption of an item to '-' if the item has submenus (and prompt the user if he wants to remove the submenu?)

For the firts part, we need to know the caption of the item that was right-clicked. I have no clue as to how to obtain that.

For the second part, I also have no clue.

Should we raise an exception in TMenuItem.Add before we fixed the menu designer (the app will probably crash at start-up, but that still is better than killing the entire OS)?

Zeljan Rikalo

2015-03-18 17:31

developer   ~0082079

IMO yes. Raise exception (with some nice message) if submenu is added to the separator menuitem.

Zeljan Rikalo

2015-03-18 17:31

developer   ~0082080

Or add lcl flag that ws supports/do not support menuitem from separator menuitem so raise exception only in case when ws does not support it, so then we're delphi compat.

Bart Broersma

2015-03-18 17:34

developer   ~0082082

> so raise exception only in case when ws does not support it
Tried that (see note 0082039) but it seems that LCLGTK2 is not defined at that time (when Menu is loaded), and using an ifdef for the disigner doesn't make sense IMO.

Zeljan Rikalo

2015-03-21 17:36

developer   ~0082166

Look into interfaces.pp -> TLCLCapability. There should be added something like lcNoSubMenuFromMenuSeparator , default as true, but in gtk2 where TLCLCapability is handled say lcNoSubMenuFromMenuSeparator = false. Then you don't need ifdef but just ask WidgetSet.HasCapability (or similar .... writing this from my mind).

Bart Broersma

2015-03-21 19:30

developer   ~0082167

Last edited: 2015-03-21 19:48

View 2 revisions

 I just fail to see why LCLGTK2 isn't defined at that point, but it is at creation time of the form (in the constructor).

I also feel your suggestion doesn't make much sense for the menu-designer....

Zeljan Rikalo

2015-03-21 21:01

developer   ~0082168

Yes it does. If ws doesn't support adding submenu to the separator menu item then just show message that it isn't supported on current widgetset.

Bart Broersma

2015-03-22 01:20

developer   ~0082173

But when you design in windows, and you want the progrsm to be cross-platform, you get no warning at all (at designtime), but a crash at runtime. Which isn't very nice.
I haven't seen a call to GetLCLCapability raise an exception upon LCL_CAPABILITY_NO (but maybe haven't looked well enough).

Bart Broersma

2015-03-22 15:43

developer   ~0082187

Updated docs to reflect the problem (r48457).

Mattias Gaertner

2015-04-05 00:07

manager   ~0082600

About: Why no IFDEF LCLGtk2 in menus.pp:
Unit menus.pp is in LCLBase. LCLBase is widgetset independent and uses an abstract widgetset.
The LCL implements the widgetset.

Bart Broersma

2015-04-05 00:38

developer   ~0082603

Last edited: 2015-04-05 00:43

View 4 revisions

 @Mattias: thanks for the clarification.

If we want to allow this on Windows (and QT-Linux (?), which at least does not disallow it, but it looks as if it is not supposed to be allowed), we have to do it in the widgetset code then?
Did anyone test GTK/QT on Windows, or other widgetsets (Carbon, Cocoa)?

I would still strongly suggest to give a warning in the menu designer (even on Windows) whenever anyone attempts to construct such a menu.

Benito van der Zander

2015-04-06 10:09

reporter   ~0082649

Does anyone even want to use a submenu on a separator?

When I submitted the bugreport, it just happened by accident. (a menu was created from a list of user defined strings. One of them was - )

A silently disappearing submenu after porting to another platform can make a program unusable.

Perhaps turn the item to a non separator, when a submenu is added?

Zeljan Rikalo

2015-04-06 11:28

developer   ~0082653

That's a solution too. Add ">" to "-" , so it becomes "->", and it's not separator anymore.

Bart Broersma

2015-04-06 11:34

developer   ~0082654

First decision that needs to be made is: do we allow it on WS that do not disallow it?
Second decision: what do we do WS where we don't allow it? Rais exception, or have a workaround as Benito suggests?
Third decision: how should the menu designer behave (disallow, warn, ask user?), and should this differ across WS?

Zeljan Rikalo

2015-04-06 17:14

developer   ~0082667

There will be complaints from windows users. Give warning about it is enough. So multiplatform developers should take care of that, and will not open issues about it.

Bart Broersma

2015-04-07 19:49

developer   ~0082701

Last edited: 2015-04-07 20:02

View 2 revisions

 Possible fix/workaround in widgetset:

class procedure TGtk2WSMenuItem.SetCaption(const AMenuItem: TMenuItem;
  const ACaption: string);
var
  MenuItemWidget: PGtkWidget;
begin
  debugln(['TGtk2WSMenuItem.SetCaption: ACaption=',ACaption,' AMenuItem.Count=',AMenuItem.Count]);
  if (ACaption = '-') and (AMenuItem.Count > 0) then
  begin
    debugln(' *** trying to set cLine on a MenuItem with childs');
    AMenuItem.Caption := '->';
  end;
...

and

class procedure TGtk2WSMenuItem.AttachMenu(const AMenuItem: TMenuItem);
var
  MenuItem, ParentMenuWidget, ContainerMenu: PGtkWidget;
begin
  debugln(['TGtk2WSMenuItem.AttachMenu: AMenuItem.Name=',AMenuItem.Name]);
  if (AMenuItem.Parent is TMenuItem) and (TMenuItem(AMenuItem.Parent).IsLine) then
  begin
    debugln(' *** attaching to a menu separator is not supported on this widgetset, adjusting parent');
    TMenuItem(AMenuItem.Parent).Caption := '->';
  end;


Or, if we want to raise an exception on such a menu:

class function TGtk2WSMenuItem.CreateHandle(const AMenuItem: TMenuItem): HMENU;
var
  Widget: PGtkWidget;
  WidgetInfo: PWidgetInfo;
begin
  debugln(['TGtk2WSMenuItem.CreateHandle: AMenuItem.Name=',AMenuItem.Name,' AMenuItem.Count=',AMenuItem.Count]);
  if AMenuItem.IsLine and (AMenuItem.Count > 0) then
    Raise Exception.Create(' *** Creating a cLine menu with submenu !!');

Zeljan Rikalo

2015-04-25 10:41

developer   ~0083202

Someone should test with r48843. Now we use gtk_separator_menu_item instead of standard menu item for separator.

Bart Broersma

2015-04-25 12:12

developer   ~0083207

Tested with r48844.
Still completely freezes my Linux as before.

Kazantsev Alexey

2015-04-25 12:45

reporter   ~0083210

I checked it on my Ubuntu 14.04 with Unity - crashes or locks is not reproduced.

Zeljan Rikalo

2015-04-25 13:08

developer   ~0083212

@Bart,better to feeeze yours than mine :))))))

Bart Broersma

2015-04-25 14:11

developer   ~0083215

> @Bart,better to feeeze yours than mine :))))))
I already thought so when I read: "Someone should test with r48843."

Zeljan Rikalo

2015-04-25 18:18

developer   ~0083217

:)))))))))))))))))

Bart Broersma

2016-01-07 11:49

developer   ~0088721

Good news: the new menu-designer disallows adding submenu's to a separator.
(Renamig the caption to '-' when the menuitem already has a submenu is still possible though, but I was told that would be fixed as well.)

Juha Manninen

2016-02-09 22:04

developer   ~0089899

The new menu-designer already disallows it. Resolving.

Issue History

Date Modified Username Field Change
2013-05-25 20:53 Benito van der Zander New Issue
2013-05-27 16:21 Zeljan Rikalo Note Added: 0067905
2015-03-15 16:41 Bart Broersma Note Added: 0081993
2015-03-15 16:43 Bart Broersma Note Edited: 0081993 View Revisions
2015-03-17 19:49 Bart Broersma Note Added: 0082039
2015-03-17 20:10 Zeljan Rikalo Note Added: 0082040
2015-03-17 21:40 Bart Broersma Note Added: 0082044
2015-03-18 08:41 Zeljan Rikalo Note Added: 0082049
2015-03-18 11:26 Bart Broersma Note Added: 0082052
2015-03-18 11:31 Zeljan Rikalo Note Added: 0082053
2015-03-18 11:37 Bart Broersma Note Added: 0082054
2015-03-18 11:43 Bart Broersma Note Added: 0082057
2015-03-18 12:55 Zeljan Rikalo Note Added: 0082065
2015-03-18 13:07 Bart Broersma Note Added: 0082068
2015-03-18 13:07 Bart Broersma Note Edited: 0082068 View Revisions
2015-03-18 17:31 Zeljan Rikalo Note Added: 0082079
2015-03-18 17:31 Zeljan Rikalo Note Added: 0082080
2015-03-18 17:34 Bart Broersma Note Added: 0082082
2015-03-21 17:36 Zeljan Rikalo Note Added: 0082166
2015-03-21 19:30 Bart Broersma Note Added: 0082167
2015-03-21 19:48 Bart Broersma Note Edited: 0082167 View Revisions
2015-03-21 21:01 Zeljan Rikalo Note Added: 0082168
2015-03-22 01:20 Bart Broersma Note Added: 0082173
2015-03-22 15:43 Bart Broersma Note Added: 0082187
2015-04-05 00:07 Mattias Gaertner Note Added: 0082600
2015-04-05 00:38 Bart Broersma Note Added: 0082603
2015-04-05 00:41 Bart Broersma Note Edited: 0082603 View Revisions
2015-04-05 00:43 Bart Broersma Note Edited: 0082603 View Revisions
2015-04-05 00:43 Bart Broersma Note Edited: 0082603 View Revisions
2015-04-06 10:09 Benito van der Zander Note Added: 0082649
2015-04-06 11:28 Zeljan Rikalo Note Added: 0082653
2015-04-06 11:34 Bart Broersma Note Added: 0082654
2015-04-06 17:14 Zeljan Rikalo Note Added: 0082667
2015-04-07 19:49 Bart Broersma Note Added: 0082701
2015-04-07 20:02 Bart Broersma Note Edited: 0082701 View Revisions
2015-04-25 10:40 Zeljan Rikalo Relationship added related to 0027905
2015-04-25 10:41 Zeljan Rikalo Note Added: 0083202
2015-04-25 12:12 Bart Broersma Note Added: 0083207
2015-04-25 12:45 Kazantsev Alexey Note Added: 0083210
2015-04-25 13:08 Zeljan Rikalo Note Added: 0083212
2015-04-25 14:11 Bart Broersma Note Added: 0083215
2015-04-25 18:18 Zeljan Rikalo Note Added: 0083217
2015-12-25 17:00 Juha Manninen Relationship added related to 0029205
2016-01-07 11:49 Bart Broersma Note Added: 0088721
2016-02-09 20:35 Juha Manninen Assigned To => Juha Manninen
2016-02-09 20:35 Juha Manninen Status new => assigned
2016-02-09 22:04 Juha Manninen LazTarget => -
2016-02-09 22:04 Juha Manninen Note Added: 0089899
2016-02-09 22:04 Juha Manninen Status assigned => resolved
2016-02-09 22:04 Juha Manninen Resolution open => fixed
2017-02-11 23:49 Juha Manninen Relationship added related to 0031138