Do bounds-checking on more windows (#4797)

Co-authored-by: pajlada <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -43,6 +43,7 @@
 - Bugfix: Fixed lookahead/-behind not working in _Ignores_. (#4965)
 - Bugfix: Fixed Image Uploader accidentally deleting images with some hosts when link resolver was enabled. (#4971)
 - Bugfix: Fixed rare crash with Image Uploader when closing a split right after starting an upload. (#4971)
+- Bugfix: Fixed some windows appearing between screens. (#4797)
 - 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/singletons/WindowManager.cpp

@@ -87,8 +87,7 @@ void WindowManager::showAccountSelectPopup(QPoint point)
 
     w->refresh();
 
-    QPoint buttonPos = point;
+    w->moveTo(point - QPoint(30, 0), widgets::BoundsChecking::CursorPosition);
-    w->move(buttonPos.x() - 30, buttonPos.y());
     w->show();
     w->setFocus();
 }

src/util/InitUpdateButton.cpp

@@ -24,7 +24,7 @@ void initUpdateButton(Button &button,
             globalPoint.setX(0);
         }
 
-        dialog->move(globalPoint);
+        dialog->moveTo(globalPoint, widgets::BoundsChecking::DesiredPosition);
         dialog->show();
         dialog->raise();
 

src/util/WidgetHelpers.cpp

@@ -79,4 +79,17 @@ void moveWindowTo(QWidget *window, QPoint position, BoundsChecking mode)
     }
 }
 
+void showAndMoveWindowTo(QWidget *window, QPoint position, BoundsChecking mode)
+{
+#ifdef Q_OS_WINDOWS
+    window->show();
+
+    moveWindowTo(window, position, mode);
+#else
+    moveWindowTo(window, position, mode);
+
+    window->show();
+#endif
+}
+
 }  // namespace chatterino::widgets

src/util/WidgetHelpers.hpp

@@ -26,4 +26,14 @@ enum class BoundsChecking {
 void moveWindowTo(QWidget *window, QPoint position,
                   BoundsChecking mode = BoundsChecking::DesiredPosition);
 
+/// Moves the `window` to the (global) `position`
+/// while doing bounds-checking according to `mode` to ensure the window stays on one screen.
+/// Will also call show on the `window`, order is dependant on platform.
+///
+/// @param window The window to move.
+/// @param position The global position to move the window to.
+/// @param mode The desired bounds checking.
+void showAndMoveWindowTo(QWidget *window, QPoint position,
+                         BoundsChecking mode = BoundsChecking::DesiredPosition);
+
 }  // namespace chatterino::widgets

src/widgets/BaseWindow.cpp

@@ -508,6 +508,11 @@ void BaseWindow::moveTo(QPoint point, widgets::BoundsChecking mode)
     widgets::moveWindowTo(this, point, mode);
 }
 
+void BaseWindow::showAndMoveTo(QPoint point, widgets::BoundsChecking mode)
+{
+    widgets::showAndMoveWindowTo(this, point, mode);
+}
+
 void BaseWindow::resizeEvent(QResizeEvent *)
 {
     // Queue up save because: Window resized
@@ -559,6 +564,12 @@ void BaseWindow::closeEvent(QCloseEvent *)
 
 void BaseWindow::showEvent(QShowEvent *)
 {
+#ifdef Q_OS_WIN
+    if (this->flags_.has(BoundsCheckOnShow))
+    {
+        this->moveTo(this->pos(), widgets::BoundsChecking::CursorPosition);
+    }
+#endif
 }
 
 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)

src/widgets/BaseWindow.hpp

@@ -27,14 +27,15 @@ class BaseWindow : public BaseWidget
 public:
     enum Flags {
         None = 0,
-        EnableCustomFrame = 1,
+        EnableCustomFrame = 1 << 0,
-        Frameless = 2,
+        Frameless = 1 << 1,
-        TopMost = 4,
+        TopMost = 1 << 2,
-        DisableCustomScaling = 8,
+        DisableCustomScaling = 1 << 3,
-        FramelessDraggable = 16,
+        FramelessDraggable = 1 << 4,
-        DontFocus = 32,
+        DontFocus = 1 << 5,
-        Dialog = 64,
+        Dialog = 1 << 6,
-        DisableLayoutSave = 128,
+        DisableLayoutSave = 1 << 7,
+        BoundsCheckOnShow = 1 << 8,
     };
 
     enum ActionOnFocusLoss { Nothing, Delete, Close, Hide };
@@ -57,6 +58,12 @@ public:
 
     void moveTo(QPoint point, widgets::BoundsChecking mode);
 
+    /**
+     * Moves the window to the given point and does bounds checking according to `mode`
+     * Depending on the platform, either the move or the show will take place first
+     **/
+    void showAndMoveTo(QPoint point, widgets::BoundsChecking mode);
+
     float scale() const override;
     float qtFontScale() const;
 

src/widgets/dialogs/ColorPickerDialog.cpp

@@ -13,7 +13,12 @@
 namespace chatterino {
 
 ColorPickerDialog::ColorPickerDialog(const QColor &initial, QWidget *parent)
-    : BasePopup({BaseWindow::EnableCustomFrame, BaseWindow::DisableLayoutSave},
+    : BasePopup(
+          {
+              BaseWindow::EnableCustomFrame,
+              BaseWindow::DisableLayoutSave,
+              BaseWindow::BoundsCheckOnShow,
+          },
           parent)
     , color_()
     , dialogConfirmed_(false)

src/widgets/dialogs/QualityPopup.cpp

@@ -9,7 +9,11 @@
 namespace chatterino {
 
 QualityPopup::QualityPopup(const QString &channelURL, QStringList options)
-    : BasePopup({BaseWindow::DisableLayoutSave},
+    : BasePopup(
+          {
+              BaseWindow::DisableLayoutSave,
+              BaseWindow::BoundsCheckOnShow,
+          },
           static_cast<QWidget *>(&(getApp()->windows->getMainWindow())))
     , channelURL_(channelURL)
 {

src/widgets/dialogs/SelectChannelDialog.cpp

@@ -31,8 +31,13 @@
 namespace chatterino {
 
 SelectChannelDialog::SelectChannelDialog(QWidget *parent)
-    : BaseWindow({BaseWindow::Flags::EnableCustomFrame,
+    : BaseWindow(
-                  BaseWindow::Flags::Dialog, BaseWindow::DisableLayoutSave},
+          {
+              BaseWindow::Flags::EnableCustomFrame,
+              BaseWindow::Flags::Dialog,
+              BaseWindow::DisableLayoutSave,
+              BaseWindow::BoundsCheckOnShow,
+          },
           parent)
     , selectedChannel_(Channel::getEmpty())
 {

src/widgets/dialogs/SettingsDialog.cpp

@@ -6,6 +6,7 @@
 #include "controllers/hotkeys/HotkeyController.hpp"
 #include "singletons/Settings.hpp"
 #include "util/LayoutCreator.hpp"
+#include "widgets/BaseWindow.hpp"
 #include "widgets/helper/Button.hpp"
 #include "widgets/helper/SettingsDialogTab.hpp"
 #include "widgets/settingspages/AboutPage.hpp"
@@ -29,8 +30,13 @@
 namespace chatterino {
 
 SettingsDialog::SettingsDialog(QWidget *parent)
-    : BaseWindow({BaseWindow::Flags::DisableCustomScaling,
+    : BaseWindow(
-                  BaseWindow::Flags::Dialog, BaseWindow::DisableLayoutSave},
+          {
+              BaseWindow::Flags::DisableCustomScaling,
+              BaseWindow::Flags::Dialog,
+              BaseWindow::DisableLayoutSave,
+              BaseWindow::BoundsCheckOnShow,
+          },
           parent)
 {
     this->setObjectName("SettingsDialog");
@@ -380,9 +386,10 @@ void SettingsDialog::themeChangedEvent()
     this->setPalette(palette);
 }
 
-void SettingsDialog::showEvent(QShowEvent *)
+void SettingsDialog::showEvent(QShowEvent *e)
 {
     this->ui_.search->setText("");
+    BaseWindow::showEvent(e);
 }
 
 ///// Widget creation helpers

src/widgets/dialogs/switcher/QuickSwitcherPopup.cpp

@@ -37,8 +37,13 @@ QList<SplitContainer *> openPages(Window *window)
 namespace chatterino {
 
 QuickSwitcherPopup::QuickSwitcherPopup(Window *parent)
-    : BasePopup({BaseWindow::Flags::Frameless, BaseWindow::Flags::TopMost,
+    : BasePopup(
-                 BaseWindow::DisableLayoutSave},
+          {
+              BaseWindow::Flags::Frameless,
+              BaseWindow::Flags::TopMost,
+              BaseWindow::DisableLayoutSave,
+              BaseWindow::BoundsCheckOnShow,
+          },
           parent)
     , switcherModel_(this)
     , window(parent)

src/widgets/helper/ChannelView.cpp

@@ -2914,8 +2914,8 @@ void ChannelView::showReplyThreadPopup(const MessagePtr &message)
     popup->setThread(message->replyThread);
 
     QPoint offset(int(150 * this->scale()), int(70 * this->scale()));
-    popup->move(QCursor::pos() - offset);
+    popup->showAndMoveTo(QCursor::pos() - offset,
-    popup->show();
+                         widgets::BoundsChecking::CursorPosition);
     popup->giveFocus(Qt::MouseFocusReason);
 }
 

src/widgets/helper/SearchPopup.cpp

@@ -63,7 +63,12 @@ ChannelPtr SearchPopup::filter(const QString &text, const QString &channelName,
 }
 
 SearchPopup::SearchPopup(QWidget *parent, Split *split)
-    : BasePopup({BaseWindow::DisableLayoutSave}, parent)
+    : BasePopup(
+          {
+              BaseWindow::DisableLayoutSave,
+              BaseWindow::BoundsCheckOnShow,
+          },
+          parent)
     , split_(split)
 {
     this->initLayout();
@@ -180,9 +185,10 @@ void SearchPopup::updateWindowTitle()
     this->setWindowTitle("Searching in " + historyName + " history");
 }
 
-void SearchPopup::showEvent(QShowEvent *)
+void SearchPopup::showEvent(QShowEvent *e)
 {
     this->search();
+    BaseWindow::showEvent(e);
 }
 
 bool SearchPopup::eventFilter(QObject *object, QEvent *event)

src/widgets/settingspages/AboutPage.cpp

@@ -227,8 +227,12 @@ void AboutPage::addLicense(QFormLayout *form, const QString &name,
     auto *b = new QLabel("<a href=\"" + licenseLink + "\">show license</a>");
     QObject::connect(
         b, &QLabel::linkActivated, [parent = this, name, licenseLink] {
-            auto window = new BasePopup({BaseWindow::Flags::EnableCustomFrame,
+            auto *window = new BasePopup(
-                                         BaseWindow::DisableLayoutSave},
+                {
+                    BaseWindow::EnableCustomFrame,
+                    BaseWindow::DisableLayoutSave,
+                    BaseWindow::BoundsCheckOnShow,
+                },
                 parent);
             window->setWindowTitle("Chatterino - License for " + name);
             window->setAttribute(Qt::WA_DeleteOnClose);

src/widgets/settingspages/FiltersPage.cpp

@@ -45,9 +45,8 @@ FiltersPage::FiltersPage()
     });
 
     // We can safely ignore this signal connection since we own the view
-    std::ignore = view->addButtonPressed.connect([] {
+    std::ignore = view->addButtonPressed.connect([this] {
-        ChannelFilterEditorDialog d(
+        ChannelFilterEditorDialog d(this->window());
-            static_cast<QWidget *>(&(getApp()->windows->getMainWindow())));
         if (d.exec() == QDialog::Accepted)
         {
             getSettings()->filterRecords.append(

src/widgets/splits/Split.cpp

@@ -192,8 +192,12 @@ namespace {
     void showTutorialVideo(QWidget *parent, const QString &source,
                            const QString &title, const QString &description)
     {
-        auto window =
+        auto *window = new BasePopup(
-            new BasePopup(BaseWindow::Flags::EnableCustomFrame, parent);
+            {
+                BaseWindow::EnableCustomFrame,
+                BaseWindow::BoundsCheckOnShow,
+            },
+            parent);
         window->setWindowTitle("Chatterino - " + title);
         window->setAttribute(Qt::WA_DeleteOnClose);
         auto layout = new QVBoxLayout();
@@ -1393,7 +1397,9 @@ void Split::showChatterList()
     multiWidget->setLayout(dockVbox);
     chatterDock->setWidget(multiWidget);
     chatterDock->setFloating(true);
-    chatterDock->show();
+    widgets::showAndMoveWindowTo(
+        chatterDock, this->mapToGlobal(QPoint{0, this->header_->height()}),
+        widgets::BoundsChecking::CursorPosition);
     chatterDock->activateWindow();
 }