search

lxqt / lxqt-panel

The LXQt desktop panel
https://lxqt-project.org
GNU Lesser General Public License v2.1
194 stars 135 forks source link

Status notifier: Tray sub-menu not shown if created immediately before showing tray menu #1417

Open chriswyatt opened 4 years ago

chriswyatt commented 4 years ago
Expected Behavior

When using the context menu (right-click) of nobleNote in the status notifier widget, it is expected that the menu items will fold out for each listed notebook

Current Behavior

Menu items for each notebook do not fold out, and appear as any other simple menu item. See https://github.com/hakaishi/nobleNote/issues/216 for more information.

Possible Solution

According to nobleNote developer, hakaishi, this may be because 'LXQt does not support dynamically changing the [sic] menu'.

Steps to Reproduce (for bugs)
  1. Install and run nobleNote
  2. Create a couple of notebooks with 2 notes in one and 3 notes in another.
  3. In the 'Status Notifier' panel widget, right-click the nobleNote icon
  4. Observe the appearance and behaviour of the menu items
Context

According to nobleNote developer, hakaishi, this works in every other environment they've tried, including KDE. For me, and from the point of view of a nobleNote user, this is a minor issue, and only means I need a few more mouse clicks to open a particular note. Although nobleNote is not a LXQt package, it is include with Lubuntu. The affected version of nobleNote is not yet included in any Lubuntu release, however.

System Information
tsujan commented 4 years ago

Qt menus don't have a property called "folding out". If it's an app property, it should be implemented correctly in the app in question.

I f you mean items cannot be added to the right click menu while it's open, that's not true. In a terminal, enter:

notify-send -a 'test1' -t 5 'Test' '<b>Test!</b>' && sleep 5 && notify-send -a 'test2' -t 5 'Test' '<b>Test!</b>' && sleep 5 && notify-send -a 'test3' -t 5 'Test' '<b>Test!</b>'

And when the notification icon appears, right click on it and wait. You'll see that items will be added to the context menu after each notification expires.

If you mean another thing, please explain it without using terms that belong to certain apps.

chriswyatt commented 4 years ago

OK, apologies. I'm not too familiar with the implementation or the terminology. I'm acting as a go between, and a rather clueless one at that! I will get more clarification and report back here.

chriswyatt commented 4 years ago

When I said 'folding out', I should have said 'submenus'.

From the nobleNote ticket:

Also, there is no problem creating the menu, only problems when updating the menu. Once updated all menus turn into unresponsive items. In other words: The submenus vanish. And we can't recreate them. Not even deleting the whole tray icon and recreating it will work. Nothing. In Qt terms: Recreating the QSystemTrayIcon will permanently destroy the QMenu's submenus. They become unreachable since the menus do not fold out anymore. Recreating the QMenu leads to the same behavior. Using the QMenu::clear() method and re-adding the QActions also leads to this behavior.

But i don't want to waste people's time here, especially if this turns out to not be a LXQt specific problem, so consider this parked until me, or someone, finds some more time to test with different DEs and distributions. Also, I'd like to try and be sure I haven't done anything stupid. I'm not familiar with any of the tools, asides from being a user of them.

tsujan commented 4 years ago

@chriswyatt I left this open in case a problem is really found or explained in a step-by-step and reproducible way, without referring to a specific app.

I've seen so many tray context menus with sub-menus and variable numbers of items (that of CMST is an example) but haven't found a problem in any of them.

That being said, according to my experience, saying "X happens only here and not anywhere else" has never been a good argument.

chriswyatt commented 4 years ago

I'm quite experienced with software testing and development, and also realise it's not a very strong argument. I wouldn't want to assume the issue lies with LXQt.

My main reason for posting here, was in case the issue seemed obvious or was perhaps even already known to any LXQt developers. I thought that it might have sped up resolution of the other ticket.

palinek commented 4 years ago

From the nobleNote ticket:

Also, there is no problem creating the menu, only problems when updating the menu. Once updated all menus turn into unresponsive items.

Can you (or the origin author of the statement) create a simple example app which will trigger the problem?

hakaishi commented 4 years ago

From the nobleNote ticket:

Also, there is no problem creating the menu, only problems when updating the menu. Once updated all menus turn into unresponsive items.

Can you (or the origin author of the statement) create a simple example app which will trigger the problem?

Here you go. :)

tsujan commented 4 years ago

@hakaishi Thanks for the code!

I confirm that, with the latest lxqt-panel, a tray submenu isn't shown if it's created immediately before the tray menu is shown (which is a valid way of adding submenus).

This seems to be our bug.

tsujan commented 4 years ago

Actually, tray submenus will be shown only if they're added before the menu is going to be shown. However, menu items (actions) are always shown, independently of how they're added.

tsujan commented 4 years ago

I found no problem in the code of Status Notifier. I think the bug is in libdbusmenu-qt5: it doesn't populate a submenu when the latter is created by connecting to QMenu::aboutToShow.

I tried to make a workaround by calling DBusMenuImporter::updateMenu before showing the menu (and showing the menu by connecting to DBusMenuImporter::menuUpdated) but it had no effect in this case because, in the attached code, the submenu is created again on showing the menu.

However, a workaround is possible in @hakaishi's code by not removing the submenu but just clearing and populating it again.

hakaishi commented 4 years ago

However, a workaround is possible in @hakaishi's code by not removing the submenu but just clearing and populating it again.

How exactly do you clear the menu? We already tried menu->clear() and re-adding the Items, but without success. We do not want to delete everything first, so our first approach was clearing. Deleting everything is quite some effort, so we would want to avoid it if possible.

tsujan commented 4 years ago

How exactly do you clear the menu?

If I don't want to remove some actions, I'll simply remove others with QWidget::removeAction and delete them if needed.

But it's your code and I just made a suggestion about the sample you provided. As I said, adding a submenu by connecting to QMenu::aboutToShow is quite valid. The problem is that, apparently, libdbusmenu-qt5 doesn't see it.

After half an hour of working on libdbusmenu-qt5, I couldn't find a way to fix the submenu issue in it (I'll work on it again if I find the time). I may have also found cases of crash, whose causes are in libdbusmenu-qt5. Therefore, I think LXQt might need to have its own version of libdbusmenu-qt5 and change it to address these issues later.

tsujan commented 4 years ago

Deleting everything is quite some effort, so we would want to avoid it if possible.

QMenu::clear() first uses QWidget::removeAction to remove each action and then, if the action only belongs to the menu, it also deletes it.

tsujan commented 1 week ago

My guess was right; the bug was in libdbusmenu-qt.

However, we don't use libdbusmenu-qt anymore and have forked in as libdbusmenu-lxqt. Having worked on our fork for another purpose, now I know why this happens. The fix is easy, and it works fine with @hakaishi's test code. Will make a PR soon.

tsujan commented 1 week ago

Sorry to give bad news: having been a little tired, I was mistaken about finding a simple fix. What I saw was just a happy illusion ;)

However, after a short rest, I checked every line of libdbusmenu-lxqt's code and found nothing wrong in it. Then I used qDebug() in it to see everything. To my great surprise, the action containing the submenu, which was created inside the slot of "aboutToShow" in the test program, didn't announce any submenu in libdbusmenu-lxqt! The method QAction::menu() simply gave null for it!

Finally, I did what I should have done from the beginning: logged into KDE and tested there. No submenu again. I didn't search for a KDE report, but the problem definitely exists in it too.

So, it seems to me that the reason should be deeper and, probably, in Qt itself.

this works in every other environment they've tried, including KDE ...

Either they made a mistake here, or it worked with an ancient version of Qt ;)