11565 confirmation dialog for reset preferences and make sure all preferences reset correctly

[krasko78]

Resolves: #11565

@cbjeukendrup @avvvvve @mathesoncalum
This PR replaces PR 25400 because I was unable to rebase due to my commits starting with "#" (a known issue it turned out) and it was easier to create a new branch and cherry-pick my changes to it. I will close 25400.

The first 24 commits are the same as those in PR 25400. The next address the code review comments left in PR 25400.
NOTES:

• As far as the dialog is concerned: whether the Cancel button will be specified as the default button or not does not make any difference now that the "Reset" button is the accented one. This means that pressing Space or Enter will execute the reset (which I personally am not fond of but at least is consistent behavior).

• The Cancel button should appear to the left of the Reset button on Mac and Linux (I didn't find any other dialog to use the ResetRole so I moved it to the right of RejectedRole)

• I've made the "First-time launch" wizard not appear after the reset which partially fixes #13299 too.

• I signed the CLA

• The title of the PR describes the problem it addresses

• Each commit's message describes its purpose and effects, and references the issue it resolves

• If changes are extensive, there is a sequence of easily reviewable commits

• The code in the PR follows the coding rules

• There are no unnecessary changes

• The code compiles and runs on my machine, preferably after each commit individually

• I created a unit test or vtest to verify the changes I made (if applicable)

[krasko78]

Whaaaaaat???? All checks have passed??? From the first time??? No kidding!!! 😂

[mathesoncalum]

Thanks for doing those tweaks @krasko78 - looks good to me now!

[avvvvve]

@krasko78 A couple UI notes (1-2) and one functionality note (3)

1. Accented button

As far as the dialog is concerned: whether the Cancel button will be specified as the default button or not does not make any difference now that the "Reset" button is the accented one. This means that pressing Space or Enter will execute the reset (which I personally am not fond of but at least is consistent behavior).

Going back on my word here. After trying this out and seeing how it feels, I agree that Space/Enter should execute the accented button AND that for that reason, we should make Cancel the accented button instead of Reset/Revert.
Would it be possible to do that while keeping the order of the buttons as-is? i.e.

• Mac: [Cancel] [Reset]
• Windows: [Reset] [Cancel]

2. Dialog title
   Both dialogs have the generic "MuseScore Studio Development" title in the top bar. For reset preferences, it should say "Reset preferences" and for factory settings it should say "Revert to factory settings"

3. Permission confirmations on Mac
   On Mac when I reset preferences, I get the macOS system dialog "MuseScore Studio would like to access files in your Documents folder". I assume resetting preferences is also wiping the permission I previously granted MuseScore to read my files so it can display previews of them in the "Home" tab. I'm wondering if that's necessary to do during "Reset preferences"—it strikes me as something we could reserve for reverting to factory settings only.

Screen.Recording.2024-12-31.at.9.54.28.AM.mov

[krasko78]

Don't you have a New Year coming guys? :)

@avvvvve:

1. No problem. I'll do this for factory reset as well.
2. I am almost 100% sure we cannot change the title of the dialog. Do you happen to have an example of such a dialog in MuseScore at hand?
3. I'll check to see what might be causing it but I am not a MacOS user so I might not be able to figure it out.

Happy New Year guys!!! :)

[cbjeukendrup]

New Year is still a few hours away (here at least)... but happy new year too!

Re. 2: technically, it's possible; see StandardDialog.qml and InteractiveProvider. It is possible to change the window title of a StyledDialogView, using its title property, but in the case of StandardDialog, this property is shadowed by the property alias title: mainPanel.title property. FWIW, the default title "MuseScore Studio [Development]" comes from DialogView::beforeOpen().

Since this PR is already quite big, I wouldn't mind if you'd do this in a separate PR, but that's up to you.

Re. 3: it is unlikely that this is because resetting preferences erases any permission settings, because those should be only accessible by the OS itself (otherwise they would be pretty useless, if apps can manage their own permissions).
I'm not sure though what is the reason for this dialog. The only thing I can think of is that QSettings on macOS uses NSUserDefaults, and perhaps NSUserDefaults is also used to identify builds (normally you would only see that dialog once per build).
Or... ISTR that for builds that are not codesigned in any way, which is the case with PR builds, you would see such a dialog for every interaction with the file system, so not once per build. So we'd need to check it in Nightly Builds (which are signed and even notarised).

It might also be interesting to figure out why the Documents folder is being touched. The best way to find out might be to set some breakpoints in filesystem.cpp, although that won't catch calls that go directly via Qt APIs. I don't think there is any OS-specificness here, so this can be done on Windows as well. It may turn out that there is a good reason for it, but it might be good to check.

[krasko78]

I can now wish you all a Happy New Year officially!!! Best wishes for everyone and their beloved ones!!! And may it bring you more frequent MuseScore Studio releases!!! :))))

Thanks Casper for the detailed comment. :)))

1. Done.
2. Done.
3. The Documents folder is accessed because MuseScore reloads the sound fonts when the Folders -> SoundFonts folder setting is reset. We don't check if a setting being reset has its default value and send a valueChanged notification unconditionally. Maybe we should optimize this?

Regarding the permission alert: Per what Casper has said, let's see if it will disappear in the nightly builds.

[avvvvve]

@krasko78 this looks great to me! And sounds good about waiting to see how the 'Documents' alert behaves in the nightlies. Not a huge deal anyway.

And happy new year to you too! ✨

[krasko78]

I am bringing back this include. QT Creator was flagging it previously as not needed and ineed everything was building and working just fine for weeks until now when all of a sudden (on my machine at least) both Qt Creator and Visual Studio can't find muse::async::Asyncable without it. Go figure.

[krasko78]

I've realized that we should probably notify the other instances about the settings that are getting reset to their default values. This will require that we fix #25323 and #24724 (they are the same).

[avvvvve]

@krasko78 do you mean you'd like to solve those issues in this PR, or do you think we could merge it as-is and solve those separately? just trying to get a sense of whether there is still work needed on this.

[krasko78]

@avvvvve I am considering fixing those issues but definitely NOT for this PR. What I was saying is that currently when you reset all prerefences and have multiple instances open, the other instances are not updated. This should probably happen from a user's perspective or maybe it is not a big deal? Also if you click the Reset to Default button on the Appearance tab of Preferences dialog for example, then all instances are updated. So there might be more work here but you guys need to decide if you want this or not. And if we do go for it, should all settings be synced or only some of them? All is much easier to implement but maybe it is undesirable to sync some settings? And finally, if we do decide to sync the theme settings, then we'll need to first fix those other issues because prevent the theme setting from getting synced.

[avvvvve]

@krasko78 Yes, resetting preferences in one instance should immediately updated preferences in every instance that's open. All settings should be synced.

This follows from the existing behavior that if you have an instance open with Preferences open, and you try to open Preferences in another instance, it just switches focus to the preferences window that's already open.

Anyway, let's merge this then if it's ready from a code-review standpoint @cbjeukendrup @mathesoncalum
And @krasko78 feel free to open a follow-up issue to make "Reset preferences" affect all open instances

[krasko78]

Ok, Ben. Thanks.

[krasko78]

Rebased and resolved the conflicts. Is there a way for all the commits not to appear as added again when rebasing? Am I doing something wrong or is this okay?

[mathesoncalum]

At first glance I thought dialogTitle looked a bit odd - it looks an alias to the title property below, which is itself an alias to mainPanel.title. Of course this isn't really the case - dialogTitle is actually an alias of PopupView::title - but I think this all highlights a problem with clashing property names that we should address.

Perhaps a less ambiguous name for the title property in StyledDialogView would actually be contentHeader - what do you think? If you agree then for the sake of consistency we should update the variable names in IInteractiveProvider, InteractiveProvider, etc.
...

Is there a way for all the commits not to appear as added again when rebasing?

Don't think so!

Outdated review - PR added some changes to standard dialogs since last review.

[krasko78]

@mathesoncalum Good catch! I didn't notice it even though it was under my nose. I like contentTitle and have updated it in all major places I think. Thanks a lot!