Don't handle profile switching when no profile is activated

[nvdaes]

Link to issue number:

Fixes #8610

Summary of the issue:

When pressing enter on normal configuration item from the profile list of profiles dialog and no profile is active, NVDA tries to handle profile switching.

Description of how this pull request fixes the issue:

In the config.ConfigManager.manualActivateProfile(), check if a profile has been changed before calling self._handleProfileSwitch().
This is done checking if is profile, since this variable is set when the active profiles are more than 1, that is, a profile different to normal is active, or when the profile to change has a name, so this doesn't happen when no profile is active and none will be changed, which is the case related to this issue.

Testing performed:

Unit tests, run NVDA from source.

Known issues with pull request:

None.
NVDA shows a dialog warning that the profile can't be activated.

Change log entry:

Section: Bug fixes.

• While no profile is active, NVDA won't try to handle profile switching when trying to activate the normal configuration.

[LeonarddeR]

What happens when you manually activated a profile, then open the configuration profile window again and press ok? Will it call _handleProfileSwitch in that case?

A proper solution would be not to call _handleProfileSwitch when the chosen profile is equal to the earlier activated profile. I don't think that this is the case in your code now, right?

[nvdaes]

leonardder commented?

What happens when you manually activated a profile, then open the configuration profile window again and press ok?

If the profile is not None, it is deactivated. If it's None, that is, if I active normal configuration, the profile is not activated again, that is, nothing occurs, but NVDA executes ConfigManager._handleProfileSwitch.
This is what happens in an add-on which I have installed, and it needs to be fixed checking if the last active profile is equal to the previously one in the config profile switch handler.
In this add-on we try to append a panel in the multicategory settings dialog when the active profile is normal configuration, else remove it.
If we open the profiles dialog when normal configuration is active and press OK, it didn't work properly
Here is code:

def handleConfigProfileSwitch(self):
self.profileName = config.conf.profiles[-1].name
if self.profileName == self.oldProfileName:
return
if not config.conf["emoticons"]["onlyNormalConfiguration"]:
deactivateAnnouncement()
self.loadDic()
if config.conf["emoticons"]["announcement"]:
activateAnnouncement()
else:
if AddonSettingsPanel not in NVDASettingsDialog.categoryClasses and self.profileName is None:
NVDASettingsDialog.categoryClasses.append(AddonSettingsPanel)
else:
try:
NVDASettingsDialog.categoryClasses.remove(AddonSettingsPanel)
except:
pass
self.oldProfileName = self.profileName

[LeonarddeR]

I've actually looked at the code now, and it seems that there is an underlying issue that causes this.

In gui/configProfiles, there are two relevant buttons, the manual activate button (self.changeStateButton) and the close button. self.changeStateButton is set as the default button, and self.AffirmativeId is set to the ID of the self.changeStateButton. This means that, even though normal configuration can be the activated profile and self.changeStateButton is disabled, enter will still trigger this button. This is obviously not what should happen.

@josephsl. You know much more about wx than I do, so you might be able to explain what is the difference between setting a particular button as a default button and setting the afirmative id of a dialog. In any case, I'd prefer having this fixed in the gui as well. Your current change makes perfect sense though, so feel free to keep that.

[nvdaes]

Thanks.
Also, if we keep my changes, when trying to activate the previously activated normal profile, a warning is shown explaining that this can't be activated; but name of normal profile is None and the dialog doesn't specify what profile is activated, since it doesn't have a name. Should we show a warning in this case?

[nvdaes]

Comparing the profiles dialog with a similar one of an add-on, I've noticed that when a profile can't be activated, profile name is not shown in the error dialog. Anyway, I think my previous question is stil valid.

Should we show a warning in this case? (when attempting to activate normal configuration).
Thanks

[LeonarddeR]

@nvdaes commented on 9 aug. 2018 08:36 CEST:

Also, if we keep my changes, when trying to activate the previously activated normal profile, a warning is shown explaining that this can't be activated; but name of normal profile is None and the dialog doesn't specify what profile is activated, since it doesn't have a name. Should we show a warning in this case?

I think I don't understand this question. You should not be able to activate a profile that is already active. For normal configuration, the change state button is disabled, and enter should no longer trigger it.

[nvdaes]

OK, if the underlying issue is fixed too, and self.AffirmativeId is not longer set to the ID of the self.changeStateButton, since this is disabled for the normal configuration, this would be fixed.
I will try this.

[nvdaes]

@LeonarddeR, seems this is not fixed changing the gui, but my last commit works as expected and the warning is not shown.
Thanks

[LeonarddeR]

@nvdaes Is this ptyhon 3 ready yet?

[nvdaes]

Hi, this is not ready, since when switching to normal configuration this should be handled.
We may close this PR or changing it.
I would like to be notified when the profile name changes, since we use different dictionaries based on the last active profile (or normal configuration) for the Emoticons add-on, by @Christianlm. I think we could add a prevConfName, similar to prevConf, to compare names and then know if dictionaries should be changed, which requires extra stuff. Now we use a global variable to keep the old and new name in profile switching
This maybe useful for future changes in NVDA. Comments are appreciated.
Cheers

[feerrenrut]

Closing this PR since there are some issues with it and it has been open a long time. In particular it looks like it will cause a bug when returning to "no profile", handleProfileSwitch will not be called. We agree the behavior described in the bug (#8610) could be improved, and would be happy to accept a PR that fixes the issue.

[nvdaes]

Perfect. 2020-07-03 9:19 GMT+02:00, Reef Turner <notifications@github.com>:

…

Closing this PR since there are some issues with it and it has been open a long time. In particular it looks like it will cause a bug when returning to "no profile", handleProfileSwitch will not be called. We agree the behavior described in the bug (#8610) could be improved, and would be happy to accept a PR that fixes the issue. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #8611 (comment)