Hide Notes action in tray icon not working on GNOME

[bjorn]

I haven't tested on any other platform yet, but the Hide Notes action in the tray icon fails to hide Notes on GNOME 43. In debugging, I noticed it actually tries to make Notes visible, because qApp->applicationState() == Qt::ApplicationInactive is true. It looks like once the tray icon is clicked, the application already goes into inactive state.

I wonder why this check is there in the first place? Ideally we can just remove it. It was added in 7478f5c so maybe @theshadowx remembers?

Note that the same check is done for the global shortcut.

[guihkx]

I wonder why this check is there in the first place?

Probably to work around a bug in either some older Qt version, or some broken Linux desktop environment.

Also, in my opinion, it seems a little odd that we're checking for so many things there:

notes/src/mainwindow.cpp

Lines 700 to 704 in 06e8f84

	// System tray context menu action: "Show/Hide Notes"
	connect(m_restoreAction, &QAction::triggered, this, [this]() {
	setMainWindowVisibility(isHidden() || windowState() == Qt::WindowMinimized
	|| (qApp->applicationState() == Qt::ApplicationInactive));
	});

I suppose it's fine to check for those when the user presses the global hotkey to summon/hide Notes, but not otherwise? 🤔

Anyway, here's my attempt at fixing this (build artifacts for linux, mac, win, if needed).

But before sending a pull request, I'd like to know if you guys agree with the above premise, of course.

[bjorn]

Anyway, here's my attempt at fixing this (build artifacts for linux, mac, win, if needed).

Did you check that indeed while minimized, isVisible() returns false? Because I do think the main window should come up from being minimized, rather than being entirely hidden, when using this action.

[guihkx]

But wouldn't that be misleading?

For example, if I minimize Notes, its tray icon would still state "Hide Notes", so why clicking on it should restore Notes? 🤔

Unless we begin to listen to minimize/unminimize events to update our tray icon label accordingly?

[bjorn]

Unless we begin to listen to minimize/unminimize events to update our tray icon label accordingly?

That would be my suggested fix for that, yeah.

[nuttyartist]

But wouldn't that be misleading?

For example, if I minimize Notes, its tray icon would still state "Hide Notes", so why clicking on it should restore Notes? 🤔

I get your argument, but I do think that if Notes is minimized and they click on the tray icon they would expect Notes to show up.

[guihkx]

I do think that if Notes is minimized and they click on the tray icon they would expect Notes to show up.

Right right, I don't disagree with that. We'll just have to update our tray icon label too. Clicking on "Hide Notes" just to have its window restored is a bit weird :p

Okay, I'll try to implement that!

[guihkx]

It will be trickier than I thought to implement this.

Both Windows and Linux fire QEvent::WindowDeactivate as soon as we right click the system tray icon.

The QEvent::WindowDeactivate event is where I was planning to update the system tray menu to say "Show Notes", and QEvent:WindowActivate is where I was planning to update the menu to say "Hide Notes".

But because right clicking the menu always deactivates the window, it's impossible for it to ever say "Hide Notes".

I thought of a workaround that would involve adding a QTimer to only update the menu after 3 seconds or so after QEvent::WindowDeactivate was fired, but that seems very hacky to me.

I'm very new at GUI programming though, so I don't know if I'm looking at this problem from the right angle.

[bjorn]

@guihkx Is there an QMenu::aboutToShow event fired for the tray icon menu? If so, that would be a good moment to update the action text I think.

[guihkx]

That's perfect, thanks! aboutToShow fires just before WindowDeactivate!

I did some tests on Plasma 5.27 and Windows 11, and the behavior on both is perfectly consistent (according to myself anyway).

However, there's this annoying bug, specific to GNOME 43, where after restoring Notes from the system tray and then focusing on some other window will, sometimes, not fire WindowDeactivate:

gnome-43-persistent-active-state-bug.mp4

Which means qApp->applicationState() == Qt::ApplicationInactive is unreliable on GNOME, because it's possible that the window gets stuck in the 'active' state wrongfully... :(

But after searching a bit, I found this interesting piece of code, which seems to be used to detect if our window is obscured by other windows:

https://github.com/bitcoin/bitcoin/blob/a245429d680eb95cf4c0c78e58e63e3f0f5d979a/src/qt/guiutil.cpp#L381-L395

And it works perfectly on GNOME! Except for that annoying "Notes is ready" notification that I get now, but that will also be gone once the focus-stealing 'feature' is implemented. 😄

Anyway, I should probably do some more testing before sending a pull request.