Bug Hunting Session
Bug 94865 - MENU: Duplicate accelerators in en_US version
Summary: MENU: Duplicate accelerators in en_US version
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
5.0.1.2 release
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyBeginner, easyHack, skillDesign, topicUI
: 97222 (view as bug list)
Depends on:
Blocks: Shortcuts-Accelerators
  Show dependency treegraph
 
Reported: 2015-10-07 15:31 UTC by sophie
Modified: 2017-02-14 08:57 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Report of accelerator duplicates in en_US version (52.05 KB, application/vnd.oasis.opendocument.text)
2015-10-07 15:31 UTC, sophie
Details
report on master (52.84 KB, application/vnd.oasis.opendocument.text)
2015-10-08 15:13 UTC, sophie
Details

Note You need to log in before you can comment on or make changes to this bug.
Description sophie 2015-10-07 15:31:50 UTC
Created attachment 119395 [details]
Report of accelerator duplicates in en_US version

Hi, following our l10n workshop in Aarhus, Niklas has created a tool that searches for duplicates of accelerators in menu/submenus. I've run it on the en_US version and attached the result to this bugs, Kendy suggested a easy-hack for it. Sophie
Comment 1 V Stuart Foote 2015-10-07 16:00:16 UTC
The accelerator auto-assign code for Recent Documents should probably reserve "C" for the "Clear List" action.  It does not currently.

Note on the attached report in ODT that some are colored yellow, and some red. Should we assume that the red colored are functionally more severe than the yellow?

Noticed many command that have no accelerator assigned are yellow--is this auto assign or .UI issue?
Comment 2 Yousuf Philips (jay) (retired) 2015-10-07 16:14:34 UTC
Sophie mentioned on IRC: those mark in red prevent to open the submenu making it an accessibility issue. yellow are for duplicates but with no submenu, so still usable but not the best to remember in a11y.

I mentioned to her because menus should have all commands in them, there will likely be duplicates

IRC Log

<sophi> jphilipz: those mark in red prevent to open the submenu making it an accessibility issue
<jphilipz> sophi: and the yellow?
<sophi> jphilipz: yellow are for duplicates but with no submenu, so still usable but not the best to remember in a11y
<jphilipz> sophi: there will always be duplicates as we have to pack all commands in the menus
<sophi> jphilipz: unless we add letters or symbols to the alphabet ;)
<jphilipz> sophi: the first one in red is Templates, and i'm able to access that by pressing T
<sophi> jphilipz: there should be another T (I didn't test it) in the menu, no?
<jphilipz> sophi: yes there is a T in export, so the problem is that if you press T the first time, it cant jump to T the second time?
<sophi> jphilipz: if there is a submenu, yes, it stays on the submenu
<jphilipz> sophi: it should have been run on master rather than an older version
<jphilipz> and 'Help Authoring' should likely have been disabled for the running
<sophi> jphilipz: accelerators are the same on master, unless they have been corrected, it's a script that place them
<jphilipz> sophi: yes changes were made to the accelerators and new entries were added into the menus
<sophi> jphilipz: I'll try to find time to install master tomorrow and will attach the new report
Comment 3 sophie 2015-10-08 15:12:46 UTC
Hi Jay, attach is the report on Version: 5.1.0.0.alpha1+
Build ID: 8273350ff48f198efc9dc9c5de5519b8cbdc0cb3. Sophie
Comment 4 sophie 2015-10-08 15:13:31 UTC
Created attachment 119424 [details]
report on master
Comment 5 Jan Holesovsky 2015-10-09 18:27:58 UTC
This is an Easy Hack: To find out where are the accelerators defined, use '~' in front of the character that has the accelerator; like when it is 'T' for 'Templates', it is '~Templates', and use git grep:

git grep '~Templates' officecfg

Open the file, and assign a better accelerator (move the ~ accordingly).

When you are done, commit & push to gerrit :-)  Thank you in advance!
Comment 6 sophie 2015-10-10 10:03:12 UTC
Hi, if the dialog has been moved to .ui file, then the accelerator is now '_', like '_Templates' in your example, so you have to grep for both accelerators.
Sophie
Comment 7 Yousuf Philips (jay) (retired) 2015-10-10 12:04:03 UTC
The menu labels are stored in the *Commands.xcu files in officecfg/registry/data/org/openoffice/Office/UI/

Accelerators in the .xcu files use '~'.

@Sophi: It would be useful to store AcceleratorKeyChecker.odt somewhere, so others can run the checking as well.
Comment 8 sophie 2015-10-12 07:57:23 UTC
Hi, Niklas has uploaded the file here:
https://www.dropbox.com/s/8e703zr5bxny27n/AcceleratorKeyChecker.odt?dl=1
Sophie
Comment 9 Robinson Tryon (qubit) 2015-12-14 06:57:03 UTC Comment hidden (obsolete)
Comment 10 Commit Notification 2015-12-22 11:31:46 UTC
Cor Nouws committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d1407130ad5ea1741d1fac14eea75f18ba119c53

tdf#94865 resolve duplicate accelerator - File>Template and File>Export

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2016-01-07 14:44:21 UTC
kumar committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=46b1988ae67d3f3ee662a0e4d0460371e321aa10

tdf#94865: Fixed MENU:Duplicate accelerators in en_US version-Patch resubmit

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 raal 2016-01-19 18:29:43 UTC
*** Bug 97222 has been marked as a duplicate of this bug. ***
Comment 13 Eike Rathke 2016-01-27 21:27:36 UTC
(In reply to sophie from comment #8)
> Hi, Niklas has uploaded the file here:
> https://www.dropbox.com/s/8e703zr5bxny27n/AcceleratorKeyChecker.odt?dl=1
> Sophie

If that document can generically check accelerator keys, could Niklas please contribute that to the code repository so it doesn't get lost and can be adapted in case changes are necessary? A storage location could be, for example, in the officecfg/util/ directory, and be mentioned with an explanation in officecfg/README

Thanks.
Comment 14 Commit Notification 2016-02-08 11:10:29 UTC
akki95 committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=775a51cec9c439cca83d0c9bac7bb7a9c66ec5c6

tdf#94865: MENU: Duplicate accelerators in en_US version

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Robinson Tryon (qubit) 2016-02-18 14:51:22 UTC Comment hidden (obsolete)
Comment 16 Pranav Ganorkar 2016-02-29 09:52:24 UTC
(In reply to sophie from comment #8)
> Hi, Niklas has uploaded the file here:
> https://www.dropbox.com/s/8e703zr5bxny27n/AcceleratorKeyChecker.odt?dl=1
> Sophie

Hi, when running the macro on 5.2.0.0.alpha0+, it fails with:

"BASIC runtime error.
Object variable not set."
Comment 17 jani 2016-04-27 06:21:25 UTC
Seems solved