From 90046f380f34ab2c00f7c239b5bf37af351fbc13 Mon Sep 17 00:00:00 2001 From: pajlada Date: Thu, 29 Dec 2022 19:15:32 +0100 Subject: [PATCH] Fix crash that would occur when performing certain actions after removing all tabs (#4271) * Ensure we can deselect notebooks * Add changelog entry * Use dynamic_cast instead of raw cast --- CHANGELOG.md | 1 + src/widgets/Notebook.cpp | 68 ++++++++++++++++++++++------------------ 2 files changed, 39 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02a8e0c32..1141d75dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Minor: Tables in settings window will now scroll to newly added rows. (#4216) - Minor: Added link to streamlink docs for easier user setup. (#4217) - Minor: Added setting to turn off rendering of reply context. (#4224) +- Bugfix: Fixed crash that would occur when performing certain actions after removing all tabs. (#4271) - Bugfix: Fixed highlight sounds not reloading on change properly. (#4194) - Bugfix: Fixed CTRL + C not working in reply thread popups. (#4209) - Bugfix: Fixed message input showing as red after removing a message that was more than 500 characters. (#4204) diff --git a/src/widgets/Notebook.cpp b/src/widgets/Notebook.cpp index 0d095da77..e797dca05 100644 --- a/src/widgets/Notebook.cpp +++ b/src/widgets/Notebook.cpp @@ -156,49 +156,50 @@ int Notebook::indexOf(QWidget *page) const void Notebook::select(QWidget *page, bool focusPage) { - if (!page) - { - return; - } - if (page == this->selectedPage_) { + // Nothing has changed return; } - auto *item = this->findItem(page); - if (!item) + if (page) { - return; - } - - page->show(); - - item->tab->setSelected(true); - item->tab->raise(); - - if (focusPage) - { - if (item->selectedWidget == nullptr) + // A new page has been selected, mark it as selected & focus one of its splits + auto *item = this->findItem(page); + if (!item) { - item->page->setFocus(); + return; } - else + + page->show(); + + item->tab->setSelected(true); + item->tab->raise(); + + if (focusPage) { - if (containsChild(page, item->selectedWidget)) + if (item->selectedWidget == nullptr) { - item->selectedWidget->setFocus(Qt::MouseFocusReason); + item->page->setFocus(); } else { - qCDebug(chatterinoWidget) << "Notebook: selected child of " - "page doesn't exist anymore"; + if (containsChild(page, item->selectedWidget)) + { + item->selectedWidget->setFocus(Qt::MouseFocusReason); + } + else + { + qCDebug(chatterinoWidget) << "Notebook: selected child of " + "page doesn't exist anymore"; + } } } } - if (this->selectedPage_ != nullptr) + if (this->selectedPage_) { + // Hide the previously selected page this->selectedPage_->hide(); auto *item = this->findItem(selectedPage_); @@ -1169,22 +1170,29 @@ SplitContainer *SplitNotebook::getOrAddSelectedPage() { auto *selectedPage = this->getSelectedPage(); - return selectedPage != nullptr ? (SplitContainer *)selectedPage - : this->addPage(); + if (selectedPage) + { + return dynamic_cast(selectedPage); + } + + return this->addPage(); } void SplitNotebook::select(QWidget *page, bool focusPage) { - if (auto selectedPage = this->getSelectedPage()) + // If there's a previously selected page, go through its splits and + // update their "last read message" indicator + if (auto *selectedPage = this->getSelectedPage()) { - if (auto splitContainer = dynamic_cast(selectedPage)) + if (auto *splitContainer = dynamic_cast(selectedPage)) { - for (auto split : splitContainer->getSplits()) + for (auto *split : splitContainer->getSplits()) { split->updateLastReadMessage(); } } } + this->Notebook::select(page, focusPage); }