Changing the synthesizer is not cancelled

CyrilleB79 commented 1 month ago

Steps to reproduce:

Start from a default configuration, i.e. with OneCore.

Open the Speech settings (NVDA+control+v)
Press the "Change..." button
Select "eSpeak NG" and validate with "OK" button
Press Escape to dismiss currently configured speech settings.

Actual behavior:

eSpeak is speaking.

Expected behavior:

Speech should be restored to OneCore.

NVDA logs, crash dumps and other attachments:

nvda.log

System configuration

NVDA installed/portable/running from source:

Installed

NVDA version:

2024.4beta4

Windows version:

Windows 10 22H2 (AMD64) build 19045.4894

Name and version of other software in use when reproducing the issue:

N/A

Other information about your system:

N/A

Other questions

Does the issue still occur after restarting your computer?

Not tested, but issue mentioned by someone else.

Have you tried any other versions of NVDA? If so, please report their behaviors.

Already present in 2019.2.1.

If NVDA add-ons are disabled, is your problem still occurring?

Yes

Does the issue still occur after you run the COM Registration Fixing Tool in NVDA's tools menu?

Not tested, probably unrelated.

seanbudd commented 1 month ago

I think this is intended behaviour, but not clearly documented in the UX. To more clearly document this, we could change this button to "Save". Alternatively, we could consider rolling back synths through the settings dialog, if that's preferred.

ruifontes commented 1 month ago

@CyrilleB79, don't forget that you can access that dialog, without passing by the NVDA configuration panel, using NVDA+Control+S... So, the synth must be changed by this dialog... Otherwise you must save the way how the dialog was open, to know if it is needed to save changes in this dialog or in the NVDA configuration panel...

CyrilleB79 commented 1 month ago

I agree that the UX can be discussed.

The fact that the synth was not restored seemed strange to me because it seems inconsistent with the following other experience:

Open speech settings (NVDA+control+V)
Change the voice, e.g. from Zira to David and note that the new voice is already speaking
Change the rate to make it much faster
Press escape and note that the original voice (Zira) and rate are restored.

The majority of options in settings dialog take effect when you press OK or apply; that's the most comman UX. There are few options that take effect as soon as the option is changed (e.g. synth settings such as voice or rate, Visual Highlight, screen curtain), but they are restored if Cancel is pressed; that's a second UX. The synth selection dialog is a third UX. Moreover, it would not be consistent either with the new URL setting dialog behaviour.

We should also note that when opening the synth selection through speech settings, we are not able to manipulate and close speech settings until the synth selection dialog is closed. That let think that the synth selection dialog, when opened through speech settings is only a way to change the synthesizer read-only field.

That's why I'd prefer to roll back the synth to the one that was previously active when one presses cancel in the speech settings.

SaschaCowley commented 1 month ago

@CyrilleB79 said

The synth selection dialog is a third UX. Moreover, it would not be consistent either with the new URL setting dialog behaviour.

I'm not sure I follow here. The new URL dialogs save their value when you press OK in the URL dialog, just as the synthesiser and braille display dialogs do.

I do agree with you that having 3 different behaviours may be confusing for users. I think there are two options (other than doing nothing):

Change the "OK" button to "Save" to make this more explicit, and/or document this behaviour more explicitly in the user guide.
Cache the value in use when opening the main settings dialog, and, if the user cancels out of the settings dialog after having made a change in a subdialog, restore the cached value.

CyrilleB79 commented 1 month ago

I'm not sure I follow here. The new URL dialogs save their value when you press OK in the URL dialog, just as the synthesiser and braille display dialogs do.

Oh yes, my bad; I had not realized that this new dialog also saves its value in the config as soon as you press OK.

Change the "OK" button to "Save" to make this more explicit, and/or document this behaviour more explicitly in the user guide.

Cache the value in use when opening the main settings dialog, and, if the user cancels out of the settings dialog after having made a change in a subdialog, restore the cached value.

The solution 2 seems more logical to me. That is: When a sub-dialog is used, it should changes the values in its parent dialog; but if Cancel is then pressed in the parent dialog, all the changes done in the parent dialog and its sub-dialogs should be discarded. This would be consistent with what is done with dictionaries: e.g. if you add an entry but finally press Cancel, the new entry is not saved to disk. We have then two sub-cases:

if the setting being changed in the sub-dialog is the synth, the change is directly applied, as are any other changes on the synth, but these changes are then reverted if the settings dialog is then discarded (Cancel pressed).
if the setting being changed in a subdialog is a mirror URL, the text field in the parent setting dialog should be updated when OK is pressed in the sub-dialog. But the actual mirror change should take effect (i.e. written in config) only if/when OK of the main settings dialog is pressed.

Note that I have not studied the case of Braille display.

On the other hand, dialogs for which the changes in themselves and sub-dialogs (such as Profiles or Add-on Store) have no OK/Cancel buttons but rather a single Close button. That makes clear that the changes done in these dialogs are immediate, i.e. cannot be reverted when the dialog is closed.

XLTechie commented 1 month ago

The Windows standard is becoming that changes are immediate. I don't think there are a lot of dialogs with sub-dialogs left, that use the OK/Cancel model in the outer most dialog.

However we still use the Cancel model to revert changes.

I don't think it is good to mix the two models. For that reason, I have to agree with Cyrille, even though that option is annoying to implement.

At least, as long as we're going to keep the current configuration mechanism. And I certainly don't see that changing this time around.

Gene703122 commented 1 month ago

I don't think this is a bug. I can't test this because I can't think of other instances of dialogs within other dialogs that have ok buttons in both dialogs to test the behavior. But it makes sense that if you activate the ok button in a dialog within a dialog, you have taken an action. the dialog closes and you are returned to the first dialog. If you cancel the first dialog, I wouldn't assume it is standard or expected Windows behavior to cancel the dialog change you made in a dialog within the outer one, which is a separate, even though inside, of the first dialog.

For as long as I can remember, until recently, the synthesizer dialog was completely separate from the speech dialog, the one that opens with NVDA control v. If users find combining the two dialogs confusing, and is there any evidence that they do, then the dialogs should be separated again.

Also, if you make cancel apply to the dialog where the action has been taken, consider the following: You have switched synthesizers in the change dialog. You are back in the speech dialog. You inadvertently set the speech speed to what you don't wish or change some other setting to something you don't want. Cancel, under this suggestion would cancel actions taken in both dialogs. That is not a valid assumption, that the user wants to change both.

Perhaps others can think of dialogs that contain ok buttons in both the first and second dialogs so the behavior can be compared, but I would be surprised if canceling the first dialog affects changes made in the dialog within the dialog. That would seem to me to be inconsistent with how Windows is structured and designed to work and I don't think NVDA should ignore or change standard Windows conventions.

Gene

wmhn1872265132 commented 1 month ago

An example for Windows:

Run "C:\Windows\System32\SystemPropertiesAdvanced.exe"
Press any setting button
Modify any options and press OK
Press Cancel

Windows also seems to save settings for subdialogs I personally have no views on this issue and the above information is shared for reference only.

CyrilleB79 commented 1 month ago

Replying to the example produced by @wmhn1872265132 in https://github.com/nvaccess/nvda/issues/17229#issuecomment-2390634850:

It's different from NVDA's experience because the value(s) modified in the child dialog does not appear in the parent dialog.
If the changes are already saved when validating the child dialog (e.g. changing an environment variable), I wonder if there is a difference if you press "OK" or "Cancel" in the main (parent) dialog...

Replying to @Gene703122's message (https://github.com/nvaccess/nvda/issues/17229#issuecomment-2389663669):

But it makes sense that if you activate the ok button in a dialog within a dialog, you have taken an action.

In my view, the action that should be considered as performed when you validate the child dialog (e.g. change synthesizer) is to change the synth value in the speech settings panel, i.e. change the value iin the read-only Synthesizer field.

the dialog closes and you are returned to the first dialog. If you cancel the first dialog, I wouldn't assume it is standard or expected Windows behavior to cancel the dialog change you made in a dialog within the outer one, which is a separate, even though inside, of the first dialog.

OK. My expectations differ from yours. No problem. That's good to hear what the expectations of various people are to figure what the most common expectation is.

For as long as I can remember, until recently, the synthesizer dialog was completely separate from the speech dialog, the one that opens with NVDA control v. If users find combining the two dialogs confusing, and is there any evidence that they do, then the dialogs should be separated again.

No. There has always been, and there is still, both the possibilities to open the synth setting dialog directly, pressing NVDA+control+s, or from the Speech settings dialog pressing the "Change" button. At least this has been the case for many years, maybe 10 years.

Also, if you make cancel apply to the dialog where the action has been taken, consider the following: You have switched synthesizers in the change dialog. You are back in the speech dialog. You inadvertently set the speech speed to what you don't wish or change some other setting to something you don't want. Cancel, under this suggestion would cancel actions taken in both dialogs. That is not a valid assumption, that the user wants to change both.

Well, that's your opinion, not mine. But, again, there is no problem to have a different opinion.

Perhaps others can think of dialogs that contain ok buttons in both the first and second dialogs so the behavior can be compared, but I would be surprised if canceling the first dialog affects changes made in the dialog within the dialog. That would seem to me to be inconsistent with how Windows is structured and designed to work and I don't think NVDA should ignore or change standard Windows conventions.

So are you disturbed by the way the NVDA dictionaries work? I.e. if you press OK in the sub-dialog to add an entry, but then press Cancel in the parent dialog to cancel all the changes made in this dialog, the new entry is not retained.

Qchristensen commented 1 month ago

Following on from the Windows example by @wmhn1872265132 - in Microsoft Office, say Word, if you open the options (alt+f, t), then tab to "Privacy settings" and open that sub-dialog, any changes you make here are saved when you press OK, even if you then escape out of Word's options dialog. Similarly in Word's options, if you control+tab to "proofing" and open AutoCorrect options, any changes there are saved on OK, and not reverted if you then escape out of the main settings dialog.

As with others, I don't have a strong preference one way or the other here, but given this issue has come up, then potentially either of the proposed alternatives may be clearer for more users?

nvaccess / nvda