From 4c1632f6a2aee25ee83e558dcfdbe56019649e30 Mon Sep 17 00:00:00 2001 From: Davide Beatrici Date: Sun, 6 Oct 2024 04:47:37 +0200 Subject: [PATCH 1/2] FIX(client): Specify version when loading/saving MainWindow state This fixes a crash caused by the state being incompatible between Qt 5 and 6. Also, by having MainWindow.ui's hash as part of the version seed we can avoid potential bugs/glitches that may arise when MainWindow's widgets are changed. (cherry picked from commit 8da6ab755cc3f8eb48b5b8fb3dbcebbffd4394be) --- cmake/project-utils.cmake | 17 ++++++++ src/mumble/CMakeLists.txt | 6 +++ src/mumble/MainWindow.cpp | 76 +++++++++++++++++++----------------- src/mumble/MainWindow.h | 5 +++ src/mumble/OverlayClient.cpp | 17 +------- 5 files changed, 70 insertions(+), 51 deletions(-) diff --git a/cmake/project-utils.cmake b/cmake/project-utils.cmake index 0b4a9fbca32..425db387858 100644 --- a/cmake/project-utils.cmake +++ b/cmake/project-utils.cmake @@ -3,6 +3,23 @@ # that can be found in the LICENSE file at the root of the # Mumble source tree or at . +# This function calculates the SHA3-256 hash of the given FILE. +# The hash is written in numerical form (sum of bytes) into HASH. +function(get_numerical_file_hash FILE HASH) + file(SHA3_256 ${FILE} HEX_HASH) + + # Split the hex hash string into pairs (each one representing a single byte). + string(REGEX MATCHALL ".." BYTES ${HEX_HASH}) + + set(NUMERICAL 0) + + foreach(BYTE ${BYTES}) + math(EXPR NUMERICAL "${NUMERICAL} + 0x${BYTE}" OUTPUT_FORMAT DECIMAL) + endforeach() + + set(${HASH} ${NUMERICAL} PARENT_SCOPE) +endfunction() + # This function gets all included subdirectories in the given DIR recursively. # It'll write the found subdirs into SUBDIRS. # Note that the DIR itself will not be added to SUBDIRS diff --git a/src/mumble/CMakeLists.txt b/src/mumble/CMakeLists.txt index fa41283bcca..e6745b5cd41 100644 --- a/src/mumble/CMakeLists.txt +++ b/src/mumble/CMakeLists.txt @@ -416,9 +416,15 @@ else() endif() endif() +# This is used to invalidate the saved widget state if a change in the source is detected. +get_numerical_file_hash("${CMAKE_CURRENT_SOURCE_DIR}/MainWindow.ui" MAINWINDOW_UI_HASH) + +message(STATUS "MAINWINDOW_UI_HASH: ${MAINWINDOW_UI_HASH}") + target_compile_definitions(mumble_client_object_lib PUBLIC "MUMBLE" + "MUMBLE_MAINWINDOW_UI_HASH=${MAINWINDOW_UI_HASH}" "QT_RESTRICTED_CAST_FROM_ASCII" ) diff --git a/src/mumble/MainWindow.cpp b/src/mumble/MainWindow.cpp index f32428a4f40..30159f1eab7 100644 --- a/src/mumble/MainWindow.cpp +++ b/src/mumble/MainWindow.cpp @@ -201,6 +201,13 @@ MainWindow::MainWindow(QWidget *p) QAccessible::installFactory(AccessibleSlider::semanticSliderFactory); } +// Loading a state that was stored by a different version of Qt can lead to a crash. +// This function calculates the state version based on Qt's version and MainWindow.ui's hash (provided through CMake). +// That way we also avoid potentially causing bugs/glitches when there are changes to MainWindow's widgets. +constexpr int MainWindow::stateVersion() { + return MUMBLE_MAINWINDOW_UI_HASH ^ QT_VERSION; +} + void MainWindow::createActions() { gsPushTalk = new GlobalShortcut(this, GlobalShortcutType::PushToTalk, tr("Push-to-Talk", "Global Shortcut")); gsPushTalk->setObjectName(QLatin1String("PushToTalk")); @@ -536,15 +543,7 @@ void MainWindow::setupGui() { setupView(false); #endif - if (Global::get().s.bMinimalView && !Global::get().s.qbaMinimalViewGeometry.isNull()) - restoreGeometry(Global::get().s.qbaMinimalViewGeometry); - else if (!Global::get().s.bMinimalView && !Global::get().s.qbaMainWindowGeometry.isNull()) - restoreGeometry(Global::get().s.qbaMainWindowGeometry); - - if (Global::get().s.bMinimalView && !Global::get().s.qbaMinimalViewState.isNull()) - restoreState(Global::get().s.qbaMinimalViewState); - else if (!Global::get().s.bMinimalView && !Global::get().s.qbaMainWindowState.isNull()) - restoreState(Global::get().s.qbaMainWindowState); + loadState(Global::get().s.bMinimalView); setupView(false); @@ -671,14 +670,7 @@ void MainWindow::closeEvent(QCloseEvent *e) { sh.reset(); - if (Global::get().s.bMinimalView) { - Global::get().s.qbaMinimalViewGeometry = saveGeometry(); - Global::get().s.qbaMinimalViewState = saveState(); - } else { - Global::get().s.qbaMainWindowGeometry = saveGeometry(); - Global::get().s.qbaMainWindowState = saveState(); - Global::get().s.qbaHeaderState = qtvUsers->header()->saveState(); - } + storeState(Global::get().s.bMinimalView); if (Global::get().talkingUI && Global::get().talkingUI->isVisible()) { // Save the TalkingUI's position if it is visible @@ -1372,6 +1364,35 @@ void MainWindow::setOnTop(bool top) { } } +void MainWindow::loadState(const bool minimalView) { + if (minimalView) { + if (!Global::get().s.qbaMinimalViewGeometry.isNull()) { + restoreGeometry(Global::get().s.qbaMinimalViewGeometry); + } + if (!Global::get().s.qbaMinimalViewState.isNull()) { + restoreState(Global::get().s.qbaMinimalViewState, stateVersion()); + } + } else { + if (!Global::get().s.qbaMainWindowGeometry.isNull()) { + restoreGeometry(Global::get().s.qbaMainWindowGeometry); + } + if (!Global::get().s.qbaMainWindowState.isNull()) { + restoreState(Global::get().s.qbaMainWindowState, stateVersion()); + } + } +} + +void MainWindow::storeState(const bool minimalView) { + if (minimalView) { + Global::get().s.qbaMinimalViewGeometry = saveGeometry(); + Global::get().s.qbaMinimalViewState = saveState(stateVersion()); + } else { + Global::get().s.qbaMainWindowGeometry = saveGeometry(); + Global::get().s.qbaMainWindowState = saveState(stateVersion()); + Global::get().s.qbaHeaderState = qtvUsers->header()->saveState(); + } +} + void MainWindow::setupView(bool toggle_minimize) { bool showit = !Global::get().s.bMinimalView; @@ -1409,14 +1430,7 @@ void MainWindow::setupView(bool toggle_minimize) { QRect geom = frameGeometry(); if (toggle_minimize) { - if (!showit) { - Global::get().s.qbaMainWindowGeometry = saveGeometry(); - Global::get().s.qbaMainWindowState = saveState(); - Global::get().s.qbaHeaderState = qtvUsers->header()->saveState(); - } else { - Global::get().s.qbaMinimalViewGeometry = saveGeometry(); - Global::get().s.qbaMinimalViewState = saveState(); - } + storeState(showit); } Qt::WindowFlags f = Qt::Window; @@ -1470,17 +1484,7 @@ void MainWindow::setupView(bool toggle_minimize) { } if (toggle_minimize) { - if (!showit) { - if (!Global::get().s.qbaMinimalViewGeometry.isNull()) - restoreGeometry(Global::get().s.qbaMinimalViewGeometry); - if (!Global::get().s.qbaMinimalViewState.isNull()) - restoreState(Global::get().s.qbaMinimalViewState); - } else { - if (!Global::get().s.qbaMainWindowGeometry.isNull()) - restoreGeometry(Global::get().s.qbaMainWindowGeometry); - if (!Global::get().s.qbaMainWindowState.isNull()) - restoreState(Global::get().s.qbaMainWindowState); - } + loadState(!showit); } else { QRect newgeom = frameGeometry(); resize(geometry().width() - newgeom.width() + geom.width(), diff --git a/src/mumble/MainWindow.h b/src/mumble/MainWindow.h index e54eab71169..da65613e98f 100644 --- a/src/mumble/MainWindow.h +++ b/src/mumble/MainWindow.h @@ -145,6 +145,9 @@ class MainWindow : public QMainWindow, public Ui::MainWindow { void focusNextMainWidget(); QPair< QByteArray, QImage > openImageFile(); + void loadState(bool minimalView); + void storeState(bool minimalView); + void updateChatBar(); void openTextMessageDialog(ClientUser *p); void openUserLocalNicknameDialog(const ClientUser &p); @@ -193,6 +196,8 @@ class MainWindow : public QMainWindow, public Ui::MainWindow { qt_unique_ptr< UserLocalVolumeSlider > m_userLocalVolumeSlider; qt_unique_ptr< ListenerVolumeSlider > m_listenerVolumeSlider; + static constexpr int stateVersion(); + void createActions(); void setupGui(); void updateWindowTitle(); diff --git a/src/mumble/OverlayClient.cpp b/src/mumble/OverlayClient.cpp index e7a8805014c..74336deb18c 100644 --- a/src/mumble/OverlayClient.cpp +++ b/src/mumble/OverlayClient.cpp @@ -206,14 +206,7 @@ void OverlayClient::showGui() { bWasVisible = !Global::get().mw->isHidden(); if (bWasVisible) { - if (Global::get().s.bMinimalView) { - Global::get().s.qbaMinimalViewGeometry = Global::get().mw->saveGeometry(); - Global::get().s.qbaMinimalViewState = Global::get().mw->saveState(); - } else { - Global::get().s.qbaMainWindowGeometry = Global::get().mw->saveGeometry(); - Global::get().s.qbaMainWindowState = Global::get().mw->saveState(); - Global::get().s.qbaHeaderState = Global::get().mw->qtvUsers->header()->saveState(); - } + Global::get().mw->storeState(Global::get().s.bMinimalView); } { @@ -315,13 +308,7 @@ void OverlayClient::hideGui() { } if (bWasVisible) { - if (Global::get().s.bMinimalView && !Global::get().s.qbaMinimalViewGeometry.isNull()) { - Global::get().mw->restoreGeometry(Global::get().s.qbaMinimalViewGeometry); - Global::get().mw->restoreState(Global::get().s.qbaMinimalViewState); - } else if (!Global::get().s.bMinimalView && !Global::get().s.qbaMainWindowGeometry.isNull()) { - Global::get().mw->restoreGeometry(Global::get().s.qbaMainWindowGeometry); - Global::get().mw->restoreState(Global::get().s.qbaMainWindowState); - } + Global::get().mw->loadState(Global::get().s.bMinimalView); } #ifdef Q_OS_MAC From 367f83cfc520ed025176cd926caa7aa8de203b60 Mon Sep 17 00:00:00 2001 From: Davide Beatrici Date: Sun, 6 Oct 2024 04:59:19 +0200 Subject: [PATCH 2/2] REFAC(client): Remove unused Settings variable (cherry picked from commit 0a03d4e1b24d92533e5d79a4b9b3798efd449520) --- src/mumble/MainWindow.cpp | 1 - src/mumble/Settings.cpp | 1 - src/mumble/Settings.h | 1 - src/mumble/SettingsMacros.h | 1 - 4 files changed, 4 deletions(-) diff --git a/src/mumble/MainWindow.cpp b/src/mumble/MainWindow.cpp index 30159f1eab7..d98691bb6d2 100644 --- a/src/mumble/MainWindow.cpp +++ b/src/mumble/MainWindow.cpp @@ -1389,7 +1389,6 @@ void MainWindow::storeState(const bool minimalView) { } else { Global::get().s.qbaMainWindowGeometry = saveGeometry(); Global::get().s.qbaMainWindowState = saveState(stateVersion()); - Global::get().s.qbaHeaderState = qtvUsers->header()->saveState(); } } diff --git a/src/mumble/Settings.cpp b/src/mumble/Settings.cpp index 0eae32d3076..ff545b0596a 100644 --- a/src/mumble/Settings.cpp +++ b/src/mumble/Settings.cpp @@ -964,7 +964,6 @@ void Settings::legacyLoad(const QString &path) { LOAD(qbaMinimalViewState, "ui/minimalviewstate"); LOAD(qbaConfigGeometry, "ui/ConfigGeometry"); LOADENUM(wlWindowLayout, "ui/WindowLayout"); - LOAD(qbaHeaderState, "ui/header"); LOAD(qsUsername, "ui/username"); LOAD(qsLastServer, "ui/server"); LOADENUM(ssFilter, "ui/serverfilter"); diff --git a/src/mumble/Settings.h b/src/mumble/Settings.h index 982af3284a3..6fd31da92ca 100644 --- a/src/mumble/Settings.h +++ b/src/mumble/Settings.h @@ -403,7 +403,6 @@ struct Settings { QByteArray qbaMainWindowState = {}; QByteArray qbaMinimalViewGeometry = {}; QByteArray qbaMinimalViewState = {}; - QByteArray qbaHeaderState = {}; QByteArray qbaConfigGeometry = {}; WindowLayout wlWindowLayout = LayoutClassic; ChannelExpand ceExpand = ChannelsWithUsers; diff --git a/src/mumble/SettingsMacros.h b/src/mumble/SettingsMacros.h index 03d5bb4449b..120a34c613f 100644 --- a/src/mumble/SettingsMacros.h +++ b/src/mumble/SettingsMacros.h @@ -158,7 +158,6 @@ PROCESS(ui, WINDOW_STATE_MINIMAL_VIEW_KEY, qbaMinimalViewState) \ PROCESS(ui, CONFIG_GEOMETRY_KEY, qbaConfigGeometry) \ PROCESS(ui, WINDOW_LAYOUT_KEY, wlWindowLayout) \ - PROCESS(ui, OVERLAY_HEADER_STATE, qbaHeaderState) \ PROCESS(ui, SERVER_FILTER_MODE_KEY, ssFilter) \ PROCESS(ui, HIDE_IN_TRAY_KEY, bHideInTray) \ PROCESS(ui, DISPLAY_TALKING_STATE_IN_TRAY_KEY, bStateInTray) \