GUIUtil::brintToFront workaround for Wayland #831

1. 
   pablomartin4btc commented at 8:51 pm on July 30, 2024: contributor

   There are known issues around handling windows focus in Wayland (this one specific in KDE but also in gnome).

   The idea is that the workaround will be executed if bitcoin-qt is running using Wayland platform (e.g.: QT_QPA_PLATFORM=wayland ./src/qt/bitcoin-qt -regtest), since the workaround behaviour looks like re-opening the window again (which I tried to fix by moving the window to the original position and/ or re-setting the original geometry without success) while in X11 (not sure in Mac) the current GUIUtil::brintToFront actually sets the focus to the desired window, keeping its original position as expected, and I didn’t want to change that (X11 behaviour).

   The solution was initially discussed with hebasto in #817.

2. 
   DrahtBot commented at 8:51 pm on July 30, 2024: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	hebasto

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   No conflicts as of last run.

3. DrahtBot added the label CI failed on Jul 30, 2024
4. 
   DrahtBot commented at 11:07 pm on July 30, 2024: contributor

   🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin-core/gui/runs/28126764186

   Make sure to run all tests locally, according to the documentation.

   The failure may happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

5. pablomartin4btc force-pushed on Jul 31, 2024
6. DrahtBot removed the label CI failed on Jul 31, 2024
7. 
   hebasto commented at 3:12 pm on August 2, 2024: member

   Concept ACK.
8. in src/qt/guiutil.cpp:1012 in bcc6874773 outdated

   1008@@ -1007,4 +1009,15 @@ void ShowModalDialogAsynchronously(QDialog* dialog)
   1009     dialog->show();
   1010 }
   1011 
   1012+void bringToFrontWorkaroundForWayland(QWidget* w)
   

   ---

   ---

   

   hebasto commented at 3:22 pm on August 2, 2024:

   Any specific reason to not inline this code?

   ---

   

   pablomartin4btc commented at 3:41 pm on August 2, 2024:

   Just to have it separately, also in case we need to touch it again.

   ---

   

   hebasto commented at 4:09 pm on August 2, 2024:

   This makes the diff bigger, as you are touching the header. Also, on Wayland, the activateWindow() and raise() calls above can be unneeded.

   ---

   

   pablomartin4btc commented at 4:18 pm on August 2, 2024:

   Ok, I’ll add a conditional there then.
9. in src/qt/guiutil.cpp:1015 in bcc6874773 outdated

   1008@@ -1007,4 +1009,15 @@ void ShowModalDialogAsynchronously(QDialog* dialog)
   1009     dialog->show();
   1010 }
   1011 
   1012+void bringToFrontWorkaroundForWayland(QWidget* w)
   1013+{
   1014+    if (QGuiApplication::platformName() == "wayland") {
   1015+        auto eFlags = w->windowFlags();
   

   ---

   ---

   

   hebasto commented at 3:23 pm on August 2, 2024:

   The variable name looks confusing to me. Why not just “flags”?

   ---

   

   pablomartin4btc commented at 3:38 pm on August 2, 2024:

   Yeah, I’ll change it.
10. pablomartin4btc force-pushed on Aug 2, 2024
11. pablomartin4btc force-pushed on Aug 2, 2024
12. in src/qt/guiutil.cpp:420 in 19f88421ac outdated

    420+            w->setWindowFlags(flags|Qt::WindowStaysOnTopHint);
    421+            w->show();
    422+            w->setWindowFlags(flags);
    423             w->show();
    424+        } else {
    425+            // activateWindow() (sometimes) helps with keyboard focus on Windows
    

    ---

    ---

    

    hebasto commented at 4:47 pm on August 2, 2024:

    Shouldn’t macOS-specific ForceActivation(); go in this branch as well?

    ---

    

    pablomartin4btc commented at 4:52 pm on August 2, 2024:

    Yeah, it makes sense.
13. gui, qt: brintToFront workaround for Wayland 15aa7d0236
14. pablomartin4btc force-pushed on Aug 2, 2024
15. hebasto approved
16. 
    hebasto commented at 2:50 pm on August 4, 2024: member

    ACK 15aa7d023688700a47997b92108de95f2d864f5a.

    Tested on Ubuntu 24.04. The behavior with QT_QPA_PLATFORM=wayland has become more consistent with the behavior with QT_QPA_PLATFORM=xcb.

    Tested on macOS 12.7.6 Monterey. No behavior changes observed.

17. DrahtBot added the label CI failed on Aug 8, 2024
18. DrahtBot removed the label CI failed on Aug 12, 2024
19. hebasto merged this on Aug 12, 2024
20. hebasto closed this on Aug 12, 2024

    ---

Contributors
pablomartin4btc DrahtBot hebasto