Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MainWindow: only perform changeEvent's hide-in-tray logic if there is a system tray available. #3025

Merged
merged 1 commit into from Apr 15, 2017

Conversation

@mkrautz
Copy link
Member

mkrautz commented Apr 15, 2017

Previously, this would hide Mumble when switching workspaces in XMonad, and possibly other WMs
without a default notification area/system tray.

See #1679

@mkrautz mkrautz requested review from hacst, Kissaki and davidebeatrici Apr 15, 2017
@Kissaki
Copy link
Member

Kissaki commented Apr 15, 2017

this would hide Mumble when switching workspaces in XMonad

QWiget::minimized()

This property is only relevant for windows.
By default, this property is false.

Uhm, given the described issue, minimized() is true on XMonad? Always? When switching to a different workspace than the Mumble window is on?

@Kissaki
Copy link
Member

Kissaki commented Apr 15, 2017

The "workaround" link is dead 😞

@mkrautz
Copy link
Member Author

mkrautz commented Apr 15, 2017

What happens is that when you switch to another workspace in XMonad is:

  1. Mumble's MainWindow gets a "spontaneous" hideEvent, the window is now hidden.
  2. Mumble's MainWindow gets a changeEvent, because of the state change (from shown to hidden)
  3. Because the window is minimized (via a spontaneous hide event), isMinimized() is now true. Mumble then invokes hide(). This causes the window to be non-visible: hide() is equivalent to setVisible(false).

This hidden state is never restored, not even for a spontaneous show event.

@mkrautz
Copy link
Member Author

mkrautz commented Apr 15, 2017

However, since there is no system tray, it isn't possible to get Mumble to show again, since it's hidden.

… a system tray available.

Previously, this would hide Mumble when switching workspaces in XMonad, and possibly other WMs
without a default notification area/system tray.

What happens when you switch to another workspace in XMonad is:

1. Mumble's MainWindow gets a "spontaneous" hideEvent. The window is now minimized.
2. Mumble's MainWindow gets a changeEvent, because of the state change from shown to minimized.
3. Because the window is minimized (via a spontaneous hide event), isMinimized() is now true.
   Mumble would then, before this change, invoke hide(). This causes the window to be non-visible:
   calling hide() is equivalent to calling setVisible(false).

This hidden state is never restored, not even for a spontaneous show event, so there is no
way, other than restarting Mumble, to get the MainWindow to show again.

This commit avoids calling hide() in changeEvent() on systems where no system tray is available,
such as XMonad.

See #1679
@mkrautz mkrautz force-pushed the mkrautz:xmonad-systray-bug branch from 0511ed0 to 224d0f3 Apr 15, 2017
@mkrautz
Copy link
Member Author

mkrautz commented Apr 15, 2017

@Kissaki updated the commit with my comments.

@Kissaki
Copy link
Member

Kissaki commented Apr 15, 2017

If XMonad calls hide (the hide event), shouldn't it also call show?
We transform a minimize into a hide gui here, but shouldn't a show restore the GUI no matter the state?

In MainWindow::hideEvent(QHideEvent *e) we already check for isSystemTrayAvailable() (for non MacOS).
What do we work around here with this code block? (explained in the dead linked forum post I guess)

@mkrautz
Copy link
Member Author

mkrautz commented Apr 15, 2017

XMonad triggers both a hideEvent() on hide, and a showEvent() on show.

However, since MainWindow is in the hidden state (hide() in changeEvent), it'll ONLY ever be shown again if you click on your system tray icon. Which isn't available in XMonad (you have to use a third-party notification area).

@mkrautz
Copy link
Member Author

mkrautz commented Apr 15, 2017

That is: the spontaneous showEvent() is a "un-minimize" event. But because we've set the window as hidden, it won't do anything.

@mkrautz mkrautz merged commit eb63d0b into mumble-voip:master Apr 15, 2017
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.