Qt: Fixing restore from system tray behaviour of main window #11859

vajdaz · 2017-12-10T00:31:16Z

When restoring the main window from the system tray, it always was in minimized state (at least on Windows). This patch should fix this. The window should restore to the same state as it was at the moment of minimizing it into the tray.

May be related to issue #8225

To reproduce the bug do following under Windows:

In Options->Window enable "Minimize to the tray instead of the taskbar"
Optionally enable "Minimize on close", too
Minimize the main window
Klick on the tray icon (or right click on the tray icon and select Show/Hide)

The main window's taskbar icon appears, but the window itself is not restored to its original size.

Expected behaviour: the window should appear.

When restoring the main window from the system tray, it always was in minimized state (at least on Windows). This patch should fix this. The window should restore to the same state as it was at the moment of minimizing it into the tray.

promag

This must be tested in linux too. BTW see #941.

promag · 2017-12-10T01:46:14Z

src/qt/bitcoingui.cpp

+                {
+                    QTimer::singleShot(0, this, SLOT(showMaximized()));
+                }
+                else if (wsevt->oldState() & Qt::WindowFullScreen)


Remove fullscreen?

promag · 2017-12-10T01:51:28Z

src/qt/bitcoingui.cpp

        }
    }
 #endif
+    QMainWindow::changeEvent(e);


I thought, it makes senses to reorder. But I see, it makes no sense. Fixed.

You could have some reason, otherwise it's unrelated to your goal.

The idea was, that theoretically we don't know what QMainWindow::changeEvent does. So if I want to add something "on top" of the default behaviour, I have to do it after that and not before. But this is kind of a academical thought. To be honest, I am not a Qt expert.

* Removing unnecessary handling of full screen state handling * Restoring original order of calling parent's changeEvent before own implementation

vajdaz · 2017-12-10T11:06:08Z

Besides Windows 10, I also have testetd on Kubuntu 16.04 (Plasma) and Ubuntu 16.04 (Gnome) . Seems to be fine to me. However, on Gnome there is no system tray. So if you minimize to tray, you loose visual reference to your application. But this is nothing that this patch introduces, it is a conceptual problem that we have already.

Failed CI test seems not to be related to my changes. Sporadic errors maybe? Could you please run the tests again?

laanwj · 2017-12-11T14:35:07Z

I've restarted the travis build.

promag · 2017-12-11T14:37:26Z

@laanwj me too, about an hour ago. Was it still failing?

laanwj · 2017-12-19T16:28:04Z

Was it still failing?

Yes.

This must be tested in linux too. BTW see #941.

Yes we've had so much problems with this feature in the past... If it turns out it's just not possible to do this reliably with Qt it'd be better to remove it.

promag · 2017-12-19T16:35:48Z

it'd be better to remove it.

Only linux, or all?

vajdaz · 2017-12-20T22:40:26Z

Meanwhile I also read a about some past tray icon issues with different OSes. Seems that Qt can't do it right everywhere.

Anyway, I have the impression, that minimizing to tray is a common thing to do on Windows. However, on Linux this is not the case (see GNOME). On Mac I don't know.

What about keeping the minimize to tray feature only on Windows?

laanwj · 2017-12-21T11:09:21Z

On Mac I don't know.

It's already disabled on MacOSX, along with the other window options:

#ifdef Q_OS_MAC
    /* remove Window tab on Mac */
    ui->tabWidget->removeTab(ui->tabWidget->indexOf(ui->tabWindow));
#endif

What about keeping the minimize to tray feature only on Windows?

Sounds good to me - but I'm not sure whether any Linux users use it, I don't at least...
Maybe we could hide it for a release first, see if anyone even notices?

jonasschnelli · 2017-12-22T20:15:50Z

Can reproduce the issue on Windows 8.1.
Tested this PR and it works,... however there is a tiny visual glich when minimising to tray (it show the main window for a couple of ms maximised after minimising), which is not really problematic.

OSX does not do minimise to tray...
needs Linux testing.

vajdaz · 2017-12-24T14:35:14Z

I have made an additional change that hides the tray icon for non-Windows systems, disables the "Hide tray icon" and "Minimize to tray" options on the Settings' Winwow tab and sets the according settings in the OptionsModel (removing them from the configuration file).

On Windows, evertything works as before. On other systems, no tray icon is visible, and no tray icon settings are available. The "Minimize on close" option still exists on all OSes.

Should I create a new PR, or should I just add those changes to this one?

laanwj · 2017-12-27T12:58:48Z

Probably best to do so as a new PR, as the change is not directly pertinent to this one.

jonasschnelli · 2018-04-10T19:21:42Z

src/qt/bitcoingui.cpp

            {
+                if (wsevt->oldState() & Qt::WindowMaximized)
+                {
+                    QTimer::singleShot(0, this, SLOT(showMaximized()));


Have you tried qApp->processEvents(); instead of the singleshot()?

jonasschnelli · 2018-04-10T19:22:08Z

Needs squash and linux testing.

laanwj · 2018-05-14T12:32:28Z

Last reply from author is some time last year, closing and adding "Up for grabs" (let me know if you start work againand I should reopen).

fanquake added the GUI label Dec 10, 2017

promag reviewed Dec 10, 2017

View reviewed changes

Adjustments on pull request bitcoin#11859

7856bb4

* Removing unnecessary handling of full screen state handling * Restoring original order of calling parent's changeEvent before own implementation

vajdaz mentioned this pull request Dec 30, 2017

Qt: Minimize to tray functionality only on Windows #12054

Closed

jonasschnelli reviewed Apr 10, 2018

View reviewed changes

laanwj added the Up for grabs label May 14, 2018

laanwj closed this May 14, 2018

hebasto mentioned this pull request Sep 15, 2018

Qt: Fix restoration of minimized to tray window #14222

Closed

maflcko removed the Up for grabs label Sep 27, 2018

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Qt: Fixing restore from system tray behaviour of main window #11859

Qt: Fixing restore from system tray behaviour of main window #11859

Uh oh!

Conversation

vajdaz commented Dec 10, 2017

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

promag Dec 10, 2017

Choose a reason for hiding this comment

Uh oh!

vajdaz Dec 10, 2017

Choose a reason for hiding this comment

Uh oh!

promag Dec 10, 2017

Choose a reason for hiding this comment

Uh oh!

vajdaz Dec 10, 2017

Choose a reason for hiding this comment

Uh oh!

promag Dec 10, 2017

Choose a reason for hiding this comment

Uh oh!

vajdaz Dec 11, 2017

Choose a reason for hiding this comment

Uh oh!

vajdaz commented Dec 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Dec 11, 2017

Uh oh!

promag commented Dec 11, 2017

Uh oh!

laanwj commented Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Dec 19, 2017

Uh oh!

vajdaz commented Dec 20, 2017

Uh oh!

laanwj commented Dec 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Dec 22, 2017

Uh oh!

vajdaz commented Dec 24, 2017

Uh oh!

laanwj commented Dec 27, 2017 via email

Uh oh!

jonasschnelli Apr 10, 2018

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Apr 10, 2018

Uh oh!

laanwj commented May 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vajdaz commented Dec 10, 2017 •

edited

Loading

laanwj commented Dec 19, 2017 •

edited

Loading

laanwj commented Dec 21, 2017 •

edited

Loading