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

Main window doesn't receive focus when Mumble-specific Windows notification is clicked #5701

Open
Snowknight26 opened this issue May 26, 2022 · 6 comments
Labels
client feature-request This issue or PR deals with a new feature help wanted Good community contribution opportunities ui windows

Comments

@Snowknight26
Copy link
Contributor

Description

Most programs that can generate a Windows notification have their main window activated whenever the notification is clicked.

Mumble, on the other hand, seems to be the only program that activates the main window in the background. As a result, if Mumble is either minimized to the system tray or its main window is not in focus, clicking on the notification does not bring it to the foreground and focus the main window.

This is especially inconvenient when a maximized window is already in focus, as after clicking on the notification you then also have to click on Mumble in the taskbar (or Alt+Tab, etc.).

I've added a video showing the reproduction steps and issue in action.

Steps to reproduce

  1. Minimize Mumble to system tray or bring another window to the foreground (so that Mumble is in the background)
  2. Click a notification that Mumble generates (such as by having someone send a message to the channel you're in)

Mumble version

1.4.230

Mumble component

Client

OS

Windows

Reproducible?

Yes

Additional information

No response

Relevant log output

No response

Screenshots

2022-05-26_00-25-03.mp4
@Snowknight26 Snowknight26 added bug A bug (error) in the software triage This issue is waiting to be triaged by one of the project members labels May 26, 2022
@Krzmbrzl
Copy link
Member

I know that I have tried bringing something to the foreground on Windows on different occasions and the result was always that it is not possible as Windows places very strict rules on when this is allowed.

See also https://stackoverflow.com/questions/5919928/qt-need-to-bring-qt-application-to-foreground-called-from-win32-application.

However, in this specific case, it might indeed work as here Mumble should be the kast application to receive an input event (the click on the notification), so I guess it would be worth a try.

@Krzmbrzl Krzmbrzl added feature-request This issue or PR deals with a new feature client ui windows good first issue Good for first-time contributors and removed bug A bug (error) in the software triage This issue is waiting to be triaged by one of the project members labels May 26, 2022
@Snowknight26
Copy link
Contributor Author

Was browsing the source code and realized that you already handle the signal that the Qt framework emits whenever the notification is clicked.

connect(qstiIcon, SIGNAL(messageClicked()), this, SLOT(showRaiseWindow()));

MainWindow::showRaiseWindow calls activateWindow, which coincidentally states:

On Windows, if you are calling this when the application is not currently the active one then it will not make it the active window. It will change the color of the taskbar entry to indicate that the window has changed in some way. This is because Microsoft does not allow an application to interrupt what the user is currently doing in another application.

Seems to be exactly what's happening, though I'm sure there has to be a way to activate the window properly seeing as other applications can. Not going to dig further though as I'm out of my depths here.

@Krzmbrzl
Copy link
Member

Ah yeah then this is exactly the behavior that I had in mind.

In this case I'll just close this issue as an "upstream bug" where "upstream" in this case is Windows itself. If even Qt themselves state that this won't work, then I don't see a big chance of use somehow circumventing Windows's restrictions here.

@Krzmbrzl Krzmbrzl added external-bug Bugs caused by things outside of our control - by dependencies, "upstream" in technical jargon and removed good first issue Good for first-time contributors labels May 30, 2022
@Snowknight26
Copy link
Contributor Author

Snowknight26 commented May 30, 2022

Not sure I agree that this is an upstream bug.

I made a test C# win forms project, created a simple form with one TextBox and one NotifyIcon.

Form class was something along the lines of (click/resize/activate/etc. event handlers simply bind to the functions outlined below so not including the rest of the class):

   public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
            notifyIcon1.Icon = Properties.Resources.Icon1;
            notifyIcon1.BalloonTipIcon = ToolTipIcon.Info;
            notifyIcon1.BalloonTipText = "Test!";
        }

        private void Form1_Resize(object sender, EventArgs e)
        {
            if(this.WindowState == FormWindowState.Minimized)
            {
                Hide();
                notifyIcon1.Visible = true;
                System.Threading.Thread.Sleep(2000);
                notifyIcon1.ShowBalloonTip(5000);
            }
        }

        private void notifyIcon1_MouseClick(object sender, MouseEventArgs e)
        {
            ActivateMainWindow();
        }

        private void notifyIcon1_BalloonTipClicked(object sender, EventArgs e)
        {
            ActivateMainWindow();
        }

        private void Form1_Activated(object sender, EventArgs e)
        {
            textBox1.Text = textBox1.Text + " " + DateTime.Now + " - " + this.Name + " activated\r\n";
        }

        private void Form1_Deactivate(object sender, EventArgs e)
        {
            textBox1.Text = textBox1.Text + " " + DateTime.Now + " - " + this.Name + " deactivated\r\n";
        }

        private void ActivateMainWindow()
        {
            Show();
            this.WindowState = FormWindowState.Normal;
            notifyIcon1.Visible = false;
        }
    }

As expected, clicking the notification that is shown when calling ShowBalloonTip() does activate the window, which illustrates that the intended behavior is definitely possible.

Digging deeper it seems that the issue might specifically be:

setWindowState((windowState() & ~Qt::WindowMinimized) | Qt::WindowActive);

If you disable the option of hiding Mumble to the system tray when minimized, minimize Mumble then clicking on a notification, Mumble restores itself and is now the top-most window (even if it's still not activated). Definitely better than before because at least now the main Mumble window is in the foreground.

I don't have a C++ compiler handy to test but I feel like showRaiseWindow needs some minor modifications, maybe something along the lines of:

void MainWindow::showRaiseWindow() {
	show();
        // don't need a conditional check for being minimized since we're always removing the Qt::WindowMinimized flag
	setWindowState((windowState() & ~Qt::WindowMinimized) | Qt::WindowActive);
	raise();
	activateWindow();
}

That should hopefully:

  1. Restore from system tray
  2. Remove the Minimized window state
  3. Set the Active window state
  4. Set the window to be the top-most window (foreground)
  5. Activate the window

@Krzmbrzl
Copy link
Member

Krzmbrzl commented May 30, 2022

I'm afraid that messing with the minimize to tray feature in any way will become an enormous undertaking in and of itself as this is a complete mess that tends to create infinite loops and whatnot 👀

It might be worth a try though

@Krzmbrzl Krzmbrzl reopened this May 30, 2022
@Krzmbrzl Krzmbrzl added help wanted Good community contribution opportunities and removed external-bug Bugs caused by things outside of our control - by dependencies, "upstream" in technical jargon labels May 30, 2022
@Snowknight26
Copy link
Contributor Author

I gave this a shot a couple weeks ago, using some ideas from other programs that use the Qt framework and correctly bring a window to focus, but didn't have any luck solely changing what showRaiseWindow does. I wonder if a handle to the foreground process/window that triggered the notification isn't being passed properly somehow. Might have to look into how and which Mumble windows create the notification.

More reading:
https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setforegroundwindow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature help wanted Good community contribution opportunities ui windows
Projects
None yet
Development

No branches or pull requests

2 participants