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

qt: Fix minimized window bug on Linux #14594

Merged
merged 1 commit into from Jan 15, 2019

Conversation

Projects
None yet
5 participants
@hebasto
Copy link
Member

commented Oct 28, 2018

Fix #14591

On some Linux systems the minimized to the taskbar (iconified) main window cannot be restored properly using actions from the systray icon menu when QSystemTrayIcon::contextMenu() is a child of the main window.

@fanquake fanquake added the GUI label Oct 28, 2018

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Oct 28, 2018

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15153 (gui: Add Open Wallet menu by promag)
  • #15149 (gui: Show current wallet name in window title by promag)
  • #15101 (gui: Add WalletController by promag)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 28, 2018

Given that this is touching the same function, and fixing similar behaviour to #14222, I'd combine this into that PR. Please keep the two commits seperate though, if these fixing different bugs etc.

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Oct 28, 2018

@fanquake it seems #14123 fixes the same issue as #14222 and the latter can be closed, if the former is merged. Do you insist on combining this PR into #14222 ?

@fanquake

This comment has been minimized.

Copy link
Member

commented Oct 28, 2018

@hebasto No that's fine, I've closed #14222.

@jonasschnelli jonasschnelli changed the title qt: Fix bug #14591 qt:Fix minimized window bug on Linux Nov 7, 2018

@hebasto hebasto changed the title qt:Fix minimized window bug on Linux qt: Fix minimized window bug on Linux Nov 13, 2018

@hebasto hebasto force-pushed the hebasto:20181028-restore-minimized branch Nov 13, 2018

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Nov 13, 2018

Rebased after #14123 merging.

@promag @jonasschnelli
Would you mind reviewing this PR?

@DrahtBot DrahtBot removed the Needs rebase label Nov 13, 2018

src/qt/guiutil.cpp Outdated
@@ -374,6 +374,12 @@ void bringToFront(QWidget* w)
// activateWindow() (sometimes) helps with keyboard focus on Windows
if (w->isMinimized()) {
w->showNormal();
// This is a workaround when the main window still minimized to the taskbar (iconified)
// due to a (possible) Qt bug or a weird window manager behaviour.
if (isObscured(w)) {

This comment has been minimized.

Copy link
@jonasschnelli

jonasschnelli Nov 25, 2018

Member

Does calling isObscured() directly after showNormal() make sense or should it hit back the the runloop first?

@hebasto hebasto force-pushed the hebasto:20181028-restore-minimized branch Nov 25, 2018

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Nov 25, 2018

@jonasschnelli
Thank you for your review and your hint making me investigate the problem more deeper.
The root of problem has been detected: X11 and/or the window manager fail to handle the main window properly when QSystemTrayIcon::contextMenu() is a child of the main window.
There is no more ugly workarounds.

Would you mind re-reviewing?

@jonasschnelli

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

utACK 40d3f19d20040ff1e0b2754fd083d0bb269c2fb5

@fanquake

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

utACK 40d3f19

src/qt/bitcoingui.cpp Outdated
@@ -224,6 +224,10 @@ BitcoinGUI::~BitcoinGUI()
settings.setValue("MainWindowGeometry", saveGeometry());
if(trayIcon) // Hide tray icon, as deleting will let it linger until quit (on Ubuntu)
trayIcon->hide();
if (trayIconMenu) {

This comment has been minimized.

Copy link
@laanwj

laanwj Jan 4, 2019

Member

We might want to use a unique_ptr and RAII here?

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

utACK (modulus nit)

@hebasto Thanks a lot for doing a thorough investigation here and not piling on more hacks that seem to do the job.

Fix minimized window bug on Linux
On some Linux systems the minimized to the taskbar (iconified) main
window cannot be restored properly using actions from the systray icon
menu when QSystemTrayIcon::contextMenu() is a child of the main window.

@hebasto hebasto force-pushed the hebasto:20181028-restore-minimized branch to a88640e Jan 4, 2019

@hebasto

This comment has been minimized.

Copy link
Member Author

commented Jan 4, 2019

@laanwj Thank you for your review.

We might want to use a unique_ptr and RAII here?

Your comment has been addressed. Would you mind re-reviewing?

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

thank you, utACK a88640e

@laanwj laanwj merged commit a88640e into bitcoin:master Jan 15, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jan 15, 2019

Merge #14594: qt: Fix minimized window bug on Linux
a88640e Fix minimized window bug on Linux (Hennadii Stepanov)

Pull request description:

  Fix #14591

  On some Linux systems the minimized to the taskbar (iconified) main window cannot be restored properly using actions from the systray icon menu when `QSystemTrayIcon::contextMenu()` is a child of the main window.

Tree-SHA512: 05c9f724fc2278d45dac6fe72b09859f12b5d71f54659bb779403c8cd81b55e610fb7b5aa912ac273d3cd19bf953b0405bbc6451feb00d1827c95dd9f0876aa4

@hebasto hebasto deleted the hebasto:20181028-restore-minimized branch Jan 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.