fix: avoid promoting child widgets to child windows (#5161)

CHANGELOG.md

@@ -79,6 +79,7 @@
 - Bugfix: Fixed splits not retaining their focus after minimizing. (#5080)
 - Bugfix: Fixed _Copy message_ copying the channel name in global search. (#5106)
 - Bugfix: Reply contexts now use the color of the replied-to message. (#5145)
+- Bugfix: Fixed top-level window getting stuck after opening settings. (#5161)
 - Dev: Run miniaudio in a separate thread, and simplify it to not manage the device ourselves. There's a chance the simplification is a bad idea. (#4978)
 - Dev: Change clang-format from v14 to v16. (#4929)
 - Dev: Fixed UTF16 encoding of `modes` file for the installer. (#4791)

src/RunGui.cpp

@@ -78,6 +78,14 @@ namespace {
     {
         // set up the QApplication flags
         QApplication::setAttribute(Qt::AA_Use96Dpi, true);
+
+#ifdef Q_OS_WIN32
+        // Avoid promoting child widgets to child windows
+        // This causes bugs with frameless windows as not all child events
+        // get sent to the parent - effectively making the window immovable.
+        QApplication::setAttribute(Qt::AA_DontCreateNativeWidgetSiblings);
+#endif
+
 #if defined(Q_OS_WIN32) && QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
         QApplication::setAttribute(Qt::AA_DisableHighDpiScaling, true);
 #endif

src/widgets/BaseWindow.cpp

@@ -201,7 +201,7 @@ void BaseWindow::init()
     }
 
 // DPI
-//    auto dpi = getWindowDpi(this->winId());
+//    auto dpi = getWindowDpi(this->safeHWND());
 
 //    if (dpi) {
 //        this->scale = dpi.value() / 96.f;
@@ -232,12 +232,13 @@ void BaseWindow::setTopMost(bool topMost)
     {
         return;
     }
+    this->isTopMost_ = topMost;
 
 #ifdef USEWINSDK
-    ::SetWindowPos(reinterpret_cast<HWND>(this->winId()),
-                   topMost ? HWND_TOPMOST : HWND_NOTOPMOST, 0, 0, 0, 0,
-                   SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
-
+    if (!this->waitingForTopMost_)
+    {
+        this->tryApplyTopMost();
+    }
 #else
     auto isVisible = this->isVisible();
     this->setWindowFlag(Qt::WindowStaysOnTopHint, topMost);
@@ -247,10 +248,26 @@ void BaseWindow::setTopMost(bool topMost)
     }
 #endif
 
-    this->isTopMost_ = topMost;
     this->topMostChanged(this->isTopMost_);
 }
 
+#ifdef USEWINSDK
+void BaseWindow::tryApplyTopMost()
+{
+    auto hwnd = this->safeHWND();
+    if (!hwnd)
+    {
+        this->waitingForTopMost_ = true;
+        QTimer::singleShot(50, this, &BaseWindow::tryApplyTopMost);
+        return;
+    }
+    this->waitingForTopMost_ = false;
+
+    ::SetWindowPos(*hwnd, this->isTopMost_ ? HWND_TOPMOST : HWND_NOTOPMOST, 0,
+                   0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
+}
+#endif
+
 bool BaseWindow::isTopMost() const
 {
     return this->isTopMost_ || this->flags_.has(TopMost);
@@ -490,14 +507,17 @@ void BaseWindow::changeEvent(QEvent *)
     }
 
     if (this->isVisible() && this->hasCustomWindowFrame())
+    {
+        auto hwnd = this->safeHWND();
+        if (hwnd)
         {
             auto palette = this->palette();
-        palette.setColor(QPalette::Window,
-                         GetForegroundWindow() == HWND(this->winId())
+            palette.setColor(QPalette::Window, GetForegroundWindow() == *hwnd
                                                    ? QColor(90, 90, 90)
                                                    : QColor(50, 50, 50));
             this->setPalette(palette);
         }
+    }
 #endif
 
 #ifndef Q_OS_WIN
@@ -532,14 +552,18 @@ void BaseWindow::resizeEvent(QResizeEvent *)
     {
         this->isResizeFixing_ = true;
         QTimer::singleShot(50, this, [this] {
+            auto hwnd = this->safeHWND();
+            if (!hwnd)
+            {
+                this->isResizeFixing_ = false;
+                return;
+            }
             RECT rect;
-            ::GetWindowRect((HWND)this->winId(), &rect);
-            ::SetWindowPos((HWND)this->winId(), nullptr, 0, 0,
-                           rect.right - rect.left + 1, rect.bottom - rect.top,
-                           SWP_NOMOVE | SWP_NOZORDER);
-            ::SetWindowPos((HWND)this->winId(), nullptr, 0, 0,
-                           rect.right - rect.left, rect.bottom - rect.top,
-                           SWP_NOMOVE | SWP_NOZORDER);
+            ::GetWindowRect(*hwnd, &rect);
+            ::SetWindowPos(*hwnd, nullptr, 0, 0, rect.right - rect.left + 1,
+                           rect.bottom - rect.top, SWP_NOMOVE | SWP_NOZORDER);
+            ::SetWindowPos(*hwnd, nullptr, 0, 0, rect.right - rect.left,
+                           rect.bottom - rect.top, SWP_NOMOVE | SWP_NOZORDER);
             QTimer::singleShot(10, this, [this] {
                 this->isResizeFixing_ = false;
             });
@@ -579,9 +603,10 @@ void BaseWindow::showEvent(QShowEvent *)
     if (!this->flags_.has(TopMost))
     {
         QTimer::singleShot(1, this, [this] {
-            ::SetWindowPos(reinterpret_cast<HWND>(this->winId()),
-                           this->isTopMost_ ? HWND_TOPMOST : HWND_NOTOPMOST, 0,
-                           0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_NOACTIVATE);
+            if (!this->waitingForTopMost_)
+            {
+                this->tryApplyTopMost();
+            }
         });
     }
 #endif
@@ -653,7 +678,7 @@ bool BaseWindow::nativeEvent(const QByteArray &eventType, void *message,
                 long y = GET_Y_LPARAM(msg->lParam);
 
                 RECT winrect;
-                GetWindowRect(HWND(winId()), &winrect);
+                GetWindowRect(msg->hwnd, &winrect);
                 QPoint globalPos(x, y);
                 this->ui_.titlebarButtons->hover(msg->wParam, globalPos);
                 this->lastEventWasNcMouseMove_ = true;
@@ -704,7 +729,7 @@ bool BaseWindow::nativeEvent(const QByteArray &eventType, void *message,
             long y = GET_Y_LPARAM(msg->lParam);
 
             RECT winrect;
-            GetWindowRect(HWND(winId()), &winrect);
+            GetWindowRect(msg->hwnd, &winrect);
             QPoint globalPos(x, y);
             if (msg->message == WM_NCLBUTTONDOWN)
             {
@@ -852,7 +877,7 @@ bool BaseWindow::handleSHOWWINDOW(MSG *msg)
         {
             // disable OS window border
             const MARGINS margins = {-1};
-            DwmExtendFrameIntoClientArea(HWND(this->winId()), &margins);
+            DwmExtendFrameIntoClientArea(msg->hwnd, &margins);
         }
 
         if (!this->initalBounds_.isNull())
@@ -888,8 +913,8 @@ bool BaseWindow::handleNCCALCSIZE(MSG *msg, long *result)
             auto *ncp = reinterpret_cast<NCCALCSIZE_PARAMS *>(msg->lParam);
             if (ncp)
             {
-                ncp->lppos->flags |= SWP_NOREDRAW;
-                ncp->rgrc[0].top -= 1;
+                // ncp->lppos->flags |= SWP_NOREDRAW;
+                // ncp->rgrc[0].top -= 1;
             }
         }
 
@@ -915,8 +940,8 @@ bool BaseWindow::handleSIZE(MSG *msg)
         {
             if (msg->wParam == SIZE_MAXIMIZED)
             {
-                auto offset = int(
-                    getWindowDpi(HWND(this->winId())).value_or(96) * 8 / 96);
+                auto offset =
+                    int(getWindowDpi(msg->hwnd).value_or(96) * 8 / 96);
 
                 this->ui_.windowLayout->setContentsMargins(offset, offset,
                                                            offset, offset);
@@ -970,7 +995,7 @@ bool BaseWindow::handleNCHITTEST(MSG *msg, long *result)
 #ifdef USEWINSDK
     const LONG border_width = 8;  // in pixels
     RECT winrect;
-    GetWindowRect(HWND(winId()), &winrect);
+    GetWindowRect(msg->hwnd, &winrect);
 
     long x = GET_X_LPARAM(msg->lParam);
     long y = GET_Y_LPARAM(msg->lParam);
@@ -1149,4 +1174,15 @@ bool BaseWindow::handleNCHITTEST(MSG *msg, long *result)
 #endif
 }
 
+#ifdef USEWINSDK
+std::optional<HWND> BaseWindow::safeHWND() const
+{
+    if (!this->testAttribute(Qt::WA_WState_Created))
+    {
+        return std::nullopt;
+    }
+    return reinterpret_cast<HWND>(this->winId());
+}
+#endif
+
 }  // namespace chatterino

src/widgets/BaseWindow.hpp

@@ -153,6 +153,25 @@ private:
     } ui_;
 
 #ifdef USEWINSDK
+    /// @brief Returns the HWND of this window if it has one
+    ///
+    /// A QWidget only has an HWND if it has been created. Before that,
+    /// accessing `winID()` will create the window which can lead to unintended
+    /// bugs.
+    std::optional<HWND> safeHWND() const;
+
+    /// @brief Tries to apply the `isTopMost_` setting
+    ///
+    /// If the setting couldn't be applied (because the window wasn't created
+    /// yet), the operation is repeated after a short delay.
+    ///
+    /// @pre When calling from outside this method, `waitingForTopMost_` must
+    ///      be `false` to avoid too many pending calls.
+    /// @post If an operation was queued to be executed after some delay,
+    ///       `waitingForTopMost_` will be set to `true`.
+    void tryApplyTopMost();
+    bool waitingForTopMost_ = false;
+
     QRect initalBounds_;
     QRect currentBounds_;
     QRect nextBounds_;