The sequence show -> raise -> activateWindow is factored out to the new function GUIUtil::bringToFront where a macOS fix is added in order to fix #13829.
Also included:
BitcoinGUI::showNormalIfMinimizedBitcoinGUI::showNormalIfMinimizedBitcoinGUI::showNormalIfMinimized.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
273@@ -274,15 +274,15 @@ void BitcoinGUI::createActions()
274 // can be triggered from the tray menu, and need to show the GUI to be useful.
275 connect(overviewAction, &QAction::triggered, this, static_cast<void (BitcoinGUI::*)()>(&BitcoinGUI::showNormalIfMinimized));
366@@ -357,6 +367,21 @@ bool isObscured(QWidget *w)
367 && checkPoint(QPoint(w->width() / 2, w->height() / 2), w));
368 }
369
370+void bringToFront(QWidget *w)
371+{
372+#ifdef Q_OS_MAC
373+ id app = objc_msgSend((id) objc_getClass("NSApplication"), sel_registerName("sharedApplication"));
374+ objc_msgSend(app, sel_getUid("activateIgnoringOtherApps:"), YES);
<objc/objc-runtime.h> #include.
@promag Can you comment here on why we need to manually do this? For reference Apples docs state:
You rarely need to invoke this method. Under most circumstances, AppKit takes care of proper activation. However, you might find this method useful if you implement your own methods for inter-app communication.
so I’d have thought this would be something handled by Qt. There are a couple mentions of this specific flag in the cocoa integration plugin, as well as use in the QCocoaWindow::raise() method.
According to the sel_getUid documentation:
The implementation of this method is identical to the implementation of
sel_registerName.
On master(750415701):
sel_registerName used 4 timessel_getUid not used at allCould sel_getUid replace with sel_registerName?
Testing current master on macOS 10.13.6 https://github.com/bitcoin/bitcoin/commit/2070a545e2c4e8c2dff4c0fb40b8fd2f68fc2ca.
If Command + H or the menu item is used to hide the window:
show/hide does not work.show (bottom of the menu ) does work.Send or Receive is clicked, the window changes to the correct tab, but the window is not shown.Sign, Verify & Debug all unhide and show the correct dialog.Testing https://github.com/bitcoin/bitcoin/pull/14123/commits/eed869b834a0d19318733420ed32f2f0df94d6be.
If Command + H or the menu item is used to hide the window:
show/hide does work.show (bottom of the menu ) does work.Send or Receive is clicked, the window changes to the correct tab, and unhides.Sign, Verify & Debug all unhide and show the correct dialog. (this has also fixed the annoying “attention” bounce behaviour that was present before.)
Thanks for the reviews! I’ll fix it.
BTW, in macOS “show/hide” action could be removed since it’s already there natively.
348@@ -349,7 +349,9 @@ void BitcoinGUI::createActions()
349 connect(encryptWalletAction, &QAction::triggered, walletFrame, &WalletFrame::encryptWallet);
350 connect(backupWalletAction, &QAction::triggered, walletFrame, &WalletFrame::backupWallet);
351 connect(changePassphraseAction, &QAction::triggered, walletFrame, &WalletFrame::changePassphrase);
352+ connect(signMessageAction, &QAction::triggered, [this]{ showNormalIfMinimized(); });
353 connect(signMessageAction, &QAction::triggered, [this]{ gotoSignMessageTab(); });
354+ connect(verifyMessageAction, &QAction::triggered, [this]{ showNormalIfMinimized(); });
355 connect(verifyMessageAction, &QAction::triggered, [this]{ gotoVerifyMessageTab(); });
I think this is better
0connect(signMessageAction, &QAction::triggered, [this]{ showNormalIfMinimized(); gotoSignMessageTab(); });
1connect(verifyMessageAction, &QAction::triggered, [this]{ showNormalIfMinimized(); gotoSignMessageTab(); });
1159- }
1160- else if(fToggleHidden)
1161+ if (!isHidden() && !isMinimized() && !GUIUtil::isObscured(this) && fToggleHidden) {
1162 hide();
1163+ } else {
1164+ GUIUtil::bringToFront(this);
showNormal.
s/show/showNormal after @ken2812221 #14123 (review).
61@@ -62,6 +62,16 @@
62 #include <QFontDatabase>
63 #endif
64
65+#if defined(Q_OS_MAC)
66+#pragma GCC diagnostic push
67+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
68+// based on: https://github.com/Mozketo/LaunchAtLoginController/blob/master/LaunchAtLoginController.m
findStartupItemInList.
373+ objc_msgSend(app, sel_getUid("activateIgnoringOtherApps:"), YES);
374+#endif
375+
376+ if (w) {
377+ // activateWindow() (sometimes) helps with keyboard focus on Windows
378+ w->activateWindow();
374+#endif
375+
376+ if (w) {
377+ // activateWindow() (sometimes) helps with keyboard focus on Windows
378+ w->activateWindow();
379+ w->showNormal();
showNormal() will break cases when then main window is maximized.
show()/showNormal() should not be aggregated with activateWindow()->raise() sequence in a single function. Rationale: show() is reverse to hide(), but showNormal() restores the widget after it has been maximized or minimized.
Qt `setWindowIcon()` does this work.
Rebased with #14597.
The e0d1883d0e98c265db09b6a48b9c124f45edf898 commit you picked has a bug: #14597 (comment) See: #14597 (comment)
59@@ -60,6 +60,15 @@
60 #include <QFontDatabase>
61 #endif
62
63+#if defined(Q_OS_MAC)
64+#pragma GCC diagnostic push
65+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
66+
67+#include <objc/objc-runtime.h>
68+#include <CoreFoundation/CoreFoundation.h>
Tested d3becb175270d715daaffb75a037433a126a95f4:
Also a tiny nit left: #14123 (review)
This moves the Dock icon click reaction code to the common place and
allows some cleanup in obj_c code.
According to the Apple's docs `class_replaceMethod` behaves as
`class_addMethod`, if the method identified by name does not yet exist;
or as `method_setImplementation`, if it does exist.
Qt `setAsDockMenu()` does this work.
Rebased to account for the new fix and applied suggestion #14123 (review).
@promag is it worth to combine a Linux-specific bugfix #14591 into this PR?
I think it is worth a new PR.
re-tACK
Systems:
Commits: 5796671e1dd8a2d0b1e750c2dce19a10af624095, 6fc21aca6d5e16c3ece104fec8e5b3df116893b4, 0a656f85a9c694f25b06c6464d6e986816eecd58