Upgraded to P3 and added a Release Note and Workaround

The fix for bug 1762 introduced a different behaviour when keyboard shortcuts are imported: these become your shortcuts, regardless of any customizations present before the import. I think this behaviour is straightforward, and what the user expects.

With this new behaviour, the steps to reproduce no longer produce a conflict, and so I think this bug can be closed.

(In reply to David Bailes from comment #2)
> The fix for bug 1762 introduced a different behaviour when keyboard
> shortcuts are imported: these become your shortcuts, regardless of any
> customizations present before the import. I think this behaviour is
> straightforward, and what the user expects.
> 
> With this new behaviour, the steps to reproduce no longer produce a
> conflict, and so I think this bug can be closed.

Please ignore the above comment - I hadn't read the steps to reproduce properly.

from the email discussion thread on the quality list:

I think I'm agreeing with David too  in that I think Option 1 is the way to go
>1. Open a message box which tells the user why the file is invalid, the 
>shortcuts are not imported, and the user has to go back and edit the xml file 
>to make it valid. 

a) It is an erroneous xml file of shortcuts
b) we should not second-guess which of the duplicated shortcuts should take
precedence and be accepted
c) rather the import of the xml shortcuts should be blocked and an error message
should be shown telling the user what the duplicates are

Additional / alternative steps to reproduce this bug (from Pavel Shostak):


1. Go to Preferences->Keyboard and set Default Standard shortcuts.
2. Export shortcuts.
3. Open the resulting xlm-file in a text editor and remove line 3 (for Open)
4. Change the shortcut in a new line 3 (for Close, ex-line 4) to "Ctrl+O".
5. Save the file.
6. Import that xml-file in Preferences->Keyboard. Now, both Open and Close have the same shortcut.

The importance of this check is that the XML file is valid (does not contain duplicates), but will currently create a duplicate keyboard shortcut in Audacity.

On Importing keyboard shortcuts, Audacity must not only check that the imported shortcuts are unique (so that the XML file is "valid"), but must also check if they conflict with existing shortcuts. The dominant opinion in the QA discussion is that imported shortcuts should override existing shortcuts.

(In reply to Steve Daulton from comment #5)
There are a few cases where duplicate shortcuts are "legal" (allowed).
These cases may be found by looking through the Full set of default key bindings.

I believe this was fixed by Binary Wisdom in commit:

https://github.com/audacity/audacity/commit/9bce51a60ec23d09de5bbf9010bd3b98892dedc9

The fix follows the agreed behavior (see emails from around October 25) . For the sake of convenience see the agreed behavior below:

_"- first, check the xml-file. If it contains illegal shortcut duplicates, refuse importing. Shortcut duplicates are LEGAL if default settings also have those operations with the matching shortcuts. A refusal to import shortcuts must happen with the message that warns the user of a failure and explains the reason.

if the xml-file looks ok, import the shortcuts. As discussed before, because different versions of Audacity might have different sets of operations with shortcuts, it is still possible to end up with illegal shortcut duplicates for a perfectly correct xml-file. This situation must be monitored. In case of any conflicts, the shortcut from the xml-file is used, and the pre-existing shortcut is wiped clean.
When telling the user the commands which have had their shortcut removed, I think it would be useful to tell the user the name of the command, the shortcut, and and name of the command which still has that shortcut."_

I didn't find a clean way to intercept the imported content before it makes its way to the shortcut preferences, so I had to jump through some hoops right in KeyConfigPrefs::OnImport().
In general, I tried to keep changes minimal.

(In reply to Leland Lucius from comment #7)
> I believe this was fixed by Binary Wisdom in commit:
> 
> https://github.com/audacity/audacity/commit/
> 9bce51a60ec23d09de5bbf9010bd3b98892dedc9
> 
> The fix follows the agreed behavior (see emails from around October 25) .
> For the sake of convenience see the agreed behavior below:
> 
> _"- first, check the xml-file. If it contains illegal shortcut duplicates,
> refuse importing. Shortcut duplicates are LEGAL if default settings also
> have those operations with the matching shortcuts. A refusal to import
> shortcuts must happen with the message that warns the user of a failure and
> explains the reason.
> 
> if the xml-file looks ok, import the shortcuts. As discussed before, because
> different versions of Audacity might have different sets of operations with
> shortcuts, it is still possible to end up with illegal shortcut duplicates
> for a perfectly correct xml-file. This situation must be monitored. In case
> of any conflicts, the shortcut from the xml-file is used, and the
> pre-existing shortcut is wiped clean.
> When telling the user the commands which have had their shortcut removed, I
> think it would be useful to tell the user the name of the command, the
> shortcut, and and name of the command which still has that shortcut."_
> 
> I didn't find a clean way to intercept the imported content before it makes
> its way to the shortcut preferences, so I had to jump through some hoops
> right in KeyConfigPrefs::OnImport().
> In general, I tried to keep changes minimal.

Apologies, I forgot to mark this as fixed. Have now done so.

Created attachment 1077 [details]
error message for duplicates

Tested on W10 with Audacity 3.0.0 accd177

This tests OK on Windows.


Note that you can't test using Gale's steps as we no longer have CheckDep, the Check Dependencies was removed a while back

(In reply to Leland Lucius from comment #7)

Tested on macOS 11.1 Big Sur with Audacity 3.0.0 accd177

This also tests OK on Mac.