Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX(client): Specify version when loading/saving MainWindow state #6584

Merged

Conversation

davidebeatrici
Copy link
Member

This fixes a crash caused by the state being incompatible between Qt 5 and 6.

Please note that the default argument value for restoreState() is 0: https://doc.qt.io/qt-6/qmainwindow.html#restoreState

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Oct 1, 2024

I would go a step further and compute a hash from the MainWindow.ui file, combine that with the Qt version and then use that as the version in the save and restore commands. That means that any change to the MainWindow should change the version, which is likely handy should we add or remove certain UI elements from it.

The hashing could be done by cmake and then be passed to the code as a macro.

@davidebeatrici
Copy link
Member Author

compute a hash from the MainWindow.ui function

You mean hashing the file itself?

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Oct 1, 2024

Yes, exactly. Such that the hash changes every time we change the UI file.

@davidebeatrici davidebeatrici changed the title FIX(client): Bump MainWindow state version to 1 for Qt 6 FIX(client): Specify version when loading/saving MainWindow state Oct 5, 2024
src/mumble/CMakeLists.txt Outdated Show resolved Hide resolved
src/mumble/MainWindow.cpp Outdated Show resolved Hide resolved
src/mumble/MainWindow.cpp Outdated Show resolved Hide resolved
src/mumble/MainWindow.cpp Show resolved Hide resolved
src/mumble/MainWindow.h Outdated Show resolved Hide resolved
src/mumble/MainWindow.cpp Outdated Show resolved Hide resolved
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.
Copy link
Member

@Hartmnt Hartmnt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I did not test it though.

@davidebeatrici davidebeatrici merged commit 9c48561 into mumble-voip:master Nov 9, 2024
23 checks passed
@davidebeatrici davidebeatrici deleted the mainwindow-state-version branch November 9, 2024 01:48
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Nov 9, 2024

💚 All backports created successfully

Status Branch Result
1.5.x

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash due to attempting to restore incompatible window state
3 participants