Only be able to add one action in a menu

glgl-schemer

I'm creating a python script to show an icon in system tray which pops a menu containing submenus, and running the script with xorg. The script contains the following snippet:

  menu = QMenu()
  for label, submenu in sorted(submenus.items()):
    sortedmenu = QMenu()
    for child in sorted(submenu, key = lambda child: child.text()):
      sortedmenu.addAction(child)
    action = item(label, 'applications-' + label.lower(), False)
    action.setMenu(sortedmenu)
    menu.addAction(action)
    print(len(menu.actions()))

  logout = lambda *_: subprocess.call(['loginctl', 'kill-session', os.getenv('XDG_SESSION_ID')])
  menu.addAction(item('Logout', 'system-log-out', logout))
  print(len(menu.actions()))

  global tray
  tray = QSystemTrayIcon()
  icon = QIcon.fromTheme('start-here')
  tray.setIcon(icon)
  tray.setVisible(True)
  tray.setContextMenu(menu)
  tray.activated.connect(on_left_click)
  app.exec()

submenus above is a dictionary, submenu is an array of actions and item creates a new action. But when I ran the script and clicked the icon, I noticed that the calls to print all printed 1, and only the last submenu added to the context menu can show, while actions in the submenu are fine. What's wrong with my code?

SGaist

Hi and welcome to devnet,

Which version of PySide/PyQt are you using ?
On which OS ?
What does the item function do ?

In any case, please provide a minimal complete script that allows to reproduce your issue.

JonB

@glgl-schemer
Also print(len(sortedmenu)), please?

glgl-schemer

I'm using pyqt6 6.5.2 on artixlinux. The following scripts just show the utilities submenu for me:

#!/usr/bin/python
import os
import subprocess
from PyQt6.QtGui import *
from PyQt6.QtWidgets import *

def on_left_click(reason):
  if reason == QSystemTrayIcon.ActivationReason.Trigger:
    tray.contextMenu().popup(QCursor.pos())

def item(label, icon, task = True):
  action = QAction(QIcon.fromTheme(icon), label)
  if task: action.triggered.connect(task)
  return action

def main():
  app = QApplication([])
  app.setQuitOnLastWindowClosed(False)

  submenus = dict()
  submenu = submenus.setdefault('Internet', [])
  submenu.append(item('firefox', 'firefox', False))
  submenu = submenus.setdefault('Utilities', [])
  submenu.append(item('firefox', 'firefox', False))
  menu = QMenu()
  for label, submenu in sorted(submenus.items()):
    sortedmenu = QMenu()
    for child in sorted(submenu, key = lambda child: child.text()):
      sortedmenu.addAction(child)
    action = item(label, 'applications-' + label.lower(), False)
    action.setMenu(sortedmenu)
    menu.addAction(action)
    print(len(menu.actions()))

  logout = lambda *_: subprocess.call(['loginctl', 'kill-session', os.getenv('XDG_SESSION_ID')])
  menu.addAction(item('Logout', 'system-log-out', logout))
  print(len(menu.actions()))

  global tray
  tray = QSystemTrayIcon()
  icon = QIcon.fromTheme('start-here')
  tray.setIcon(icon)
  tray.setVisible(True)
  tray.setContextMenu(menu)
  tray.activated.connect(on_left_click)
  app.exec()

if __name__ == '__main__':
  main()

@JonB The contents in submenus are fine, but only the last menu I added into the root menu survives. I updated a complete script that can reproduce the problem for me.

StarterKit

@glgl-schemer, your problem lies in how item() function works with action variable.
I modified your code a bit, try it and compare its behavior with your code:

  submenus = {
    'Internet': [item('firefox', 'firefox', False)],
    'Utilities': [item('firefox', 'firefox', False)]
  }
  actions = []    # <----- this line was added  -----------------
  menu = QMenu()
  for label, submenu in sorted(submenus.items()):
    sortedmenu = QMenu()
    for child in sorted(submenu, key = lambda child: child.text()):
      sortedmenu.addAction(child)
    action = item(label, 'applications-' + label.lower(), False)
    action.setMenu(sortedmenu)
    actions.append(action)  # <----- this line was added  ---------------
    menu.addAction(action)
    print(len(menu.actions()))

This is how python garbage collector works - when you assign action second time in your code you lose a reference to this object. As result it is destroyed by python garbage collector as not used anymore and you lose a menu item.
You need to keep refrences to all objects that are still alive in your GUI.

JonB

@StarterKit
I don't doubt that your new code works, but I personally do not understand why it is necessary. I know about Python garbage collection and references, though not a 100% expert. Given that the code goes menu.addAction(action) I believe that takes ownership(?) and do not see what actions.append(action) then adds to this?

OOH, update: void QWidget::addAction(QAction *action)

The ownership of action is not transferred to this QWidget.

That would indeed make a difference!

Then instead of some global actions list wouldn't it be better to pass a parent to QAction::QAction(QObject *parent = nullptr) constructor? What is that item(...) method, where in docs, please?

I don't know whether it is relevant, but existing code

action.setMenu(sortedmenu)
menu.addAction(action)

I get confused, why are two different QMenus used here? But I may just be forgetting/misunderstanding how this works.

StarterKit

Hi @JonB, yes you quotation from Qt docs is exactly right.
I just want to add that it isn't only about QWidget::addAction() - there are other places where Qt doesn't take an ownership of an object. So it is better to always keep an eye on it.
With regards to the code - I tried to make just a simple adjustment to highlight the problem with a spot light. I also a bit confused with how code is structured but... this is only an example and who am I to teach others to code? :)

JonB

@StarterKit
I asked:

What is that item(...) method, where in docs, please?

for the action = item(label, 'applications-' + label.lower(), False) statement.

StarterKit

@JonB it isn't a Qt method.
This is a method from the code provided by the topic starter, it makes an action object from label and icon data:

def item(label, icon, task = True):
  action = QAction(QIcon.fromTheme(icon), label)
  if task: action.triggered.connect(task)
  return action

JonB

@StarterKit
Sorry, only looked at his original post! Thought it was some Qt or Python thing, no wonder I didn't follow! Got it now!

Still seems to me that the action = QAction(QIcon.fromTheme(icon), label) does have a parent/owner of the passed in label, and that is an actual QLabel in the UI hierarchy. So that should be enough to prevent garbage collection, still not sure why your actions is needed in this case....

Hey, wait a minute!

StarterKit

@JonB, as I see label isn't a QLabel instance here. It is just a piece of text.

I feel you might be right and there might be a chance that if we put proper parent in every constructor call then we wouldn't need more tricks. But I still not fully sure and I defninitely know that there are cases when such things are not possible due to lack of parents :)

JonB

@StarterKit
Just spotted that:

action = QAction(QIcon.fromTheme(icon), label)

That must be QAction::QAction(const QIcon &icon, const QString &text, QObject *parent = nullptr). So label must be label text not QLabel??

So pass s suitable QWidget here as parent parameter, then you shouldn't need that actions list....

glgl-schemer

@StarterKit Thanks. After holding actions and sorted menus in arrays, my script works as expected.

StarterKit

@JonB said in Only be able to add one action in a menu:

That must be QAction::QAction(const QIcon &icon, const QString &text, QObject *parent = nullptr). So label must be label text not QLabel??

This is exactly the problem. For this particular case Qt allows to constract QAction either with QLabel or with simple text. Similar situation was in my case that led to multiple seg-faults - Qt allows objects to be constructed with None parent but such an object may be lost occasionally and then access to it causes segmentation violation.

Would Qt be more strict with such things these problem won't exist. But... we are with Python here - who cases about strict typization and other strict things?... I don't know the right answer... most probably tools should evolve. I like Python for its flexibility but at the same time hate it for the lack of strict types. So... next version should be better, shoudn't it? :)