Merge PR mumble-voip#6584: FIX(client): Specify version when loading/…

…saving MainWindow state
Hartmnt · Nov 9, 2024 · 9c48561 · 9c48561
2 parents c7d902b + 0a03d4e
commit 9c48561
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 54 deletions.
diff --git 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 <https://www.mumble.info/LICENSE>.
 
+# 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
@@ -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
@@ -200,6 +200,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"));
@@ -544,15 +551,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);
 
@@ -679,14 +678,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
@@ -1380,6 +1372,34 @@ 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());
+	}
+}
+
 void MainWindow::setupView(bool toggle_minimize) {
 	bool showit = !Global::get().s.bMinimalView;
 
@@ -1417,14 +1437,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;
@@ -1478,17 +1491,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
@@ -146,6 +146,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);
@@ -194,6 +197,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
@@ -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

diff --git a/src/mumble/Settings.cpp b/src/mumble/Settings.cpp
@@ -951,7 +951,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
@@ -402,7 +402,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
@@ -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)                 \