Bug 151442 - [UI] Windows menu item (Sheet, Delete Rows) has a non-unique accelerator
Summary: [UI] Windows menu item (Sheet, Delete Rows) has a non-unique accelerator
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
7.4.1.2 release
Hardware: All All
: medium minor
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: bibisectNotNeeded, regression
: 152607 (view as bug list)
Depends on:
Blocks: Shortcuts-Accelerators
  Show dependency treegraph
 
Reported: 2022-10-09 23:55 UTC by Sean Gugler
Modified: 2023-09-30 04:31 UTC (History)
9 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sean Gugler 2022-10-09 23:55:16 UTC
Description:
In Calc for Windows, the menu item "Sheet > Delete Rows" cannot be invoked with keyboard accelerators because it is assigned the same shortcut ("R") as an earlier item in the same menu ("Insert Rows").

Steps to Reproduce:
1. Open the Calc application, with any document or a new document
2. Press the keyboard combination Alt+S
3. Press the key R
4. Press the key R again

Actual Results:
The "Delete Rows" menu item is not selected, nor acted upon.

Expected Results:
The "Delete Rows" menu item should be assigned a unique accelerator so that it may be quickly invoked from the keyboard with the assistance of the Alt key.


Reproducible: Always


User Profile Reset: No



Additional Info:
This is a regression from 7.3.5 in which "Delete Rows" was assigned the unique accelerator "W". My muscle memory grew accustomed to the key sequence "Alt+S, W" to delete selected rows. I don't necessarily object to re-training myself when an improvement is made, but the current sequence "Alt+S, down-arrow (x5), Enter" is not an improvement.
Comment 1 raal 2022-10-10 04:35:22 UTC
Confirm with Version: 7.5.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 07aa8138db9bbaf222f2b7cea12a9f7d0a8192d7
CPU threads: 4; OS: Windows 10.0 Build 19044; UI render: Skia/Raster; VCL: win
Locale: cs-CZ (cs_CZ); UI: en-US
Calc: CL threaded
Comment 2 raal 2022-10-10 04:55:08 UTC
bisected (accelerator O, so it changed twice) to c03cd36e5dad3fffa086c4625a30d08e70a34237 is the first bad commit
commit c03cd36e5dad3fffa086c4625a30d08e70a34237
Author: Norbert Thiebaud <nthiebaud@gmail.com>
Date:   Thu Mar 17 01:37:04 2022 -0700

    source 763e5d233d18a26f64dd8c4db67ce62ee289832e

@Julien, not exact commit, but please can you fix it? Thank you
Comment 3 Julien Nabet 2022-10-10 06:12:50 UTC
On pc Debian x86-64 with master sources updated today, I could reproduce this but don't know why it uses "R".
I took a look at https://cgit.freedesktop.org/libreoffice/core/commit/?id=763e5d233d18a26f64dd8c4db67ce62ee289832e
and don't know why "R" is selected.

I also took a look at Customize menu, I didn't find a way to change the shortcut.

Don't hesitate to revert the patch but it would be great to find another way.

Heiko/Xisco: any thoughts here?
Comment 4 Heiko Tietze 2022-10-10 08:11:04 UTC
(In reply to Julien Nabet from comment #3)
> Heiko/Xisco: any thoughts here?

IIRC, we have functions that assign the mnemonic automatically. At least for the UI files it's not needed anymore.

It's Sheet > Delete R~ows for me on kf5 / 7.4.

In case of more than one mnemonic the expected behavior is to select/highlight the item and iterate over it but execute per enter/space. Meaning alt > S > R > R > R... jumps from "Insert ~Row" to "Delete ~Row" and back, for example - don't know the actual hotkeys.
Comment 5 Sean Gugler 2022-10-10 19:00:05 UTC
The first "R" will select "Insert Rows", and since that item has a sub-menu, it cascades open. Subsequent "R" does not jump to "Delete Rows" at the higher level. If there were an "R" assigned in the sub-menu I presume that item would be activated, but since there isn't, pressing "R" has no further effect. It does not return focus to the higher-level menu where "Delete Rows" is found, as you might expect.

It seems that, in case of more than one mnemonic, the logic design does not take sub-menus into account.

The algorithm that automatically assigns accelerator keys must have changed since 7.3.5 when "Delete Rows" was given the unique hotkey "W". I liked it when it picked unique letters; is it hard to go back?
Comment 6 Heiko Tietze 2022-10-11 06:56:14 UTC
(In reply to Sean Gugler from comment #5)
> It seems that, in case of more than one mnemonic, the logic design does not
> take sub-menus into account.

Not sure that we handle this or the OS/DE. But thanks for looking into it.

> The algorithm that automatically assigns accelerator keys must have changed
> since 7.3.5 when "Delete Rows" was given the unique hotkey "W". I liked it
> when it picked unique letters; is it hard to go back?

AFAICS, the mnemonic is hard-coded. But you can try to modify via tools > customize > menu: find the command at the right-hand list (use the dropdown above for the root item), and set the hotkey with a tilde per Modify > Rename. For example "Insert ~Rows".
Comment 7 Ming Hua 2022-10-11 08:35:25 UTC
(In reply to Julien Nabet from comment #3)
> On pc Debian x86-64 with master sources updated today, I could reproduce
> this but don't know why it uses "R".
> I took a look at
> https://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=763e5d233d18a26f64dd8c4db67ce62ee289832e
> and don't know why "R" is selected.

(In reply to Heiko Tietze from comment #4)
> IIRC, we have functions that assign the mnemonic automatically. At least for
> the UI files it's not needed anymore.
> 
> It's Sheet > Delete R~ows for me on kf5 / 7.4.

I'm merely a translator and don't know much about the automatic mnemonic generation mechanism, but I'm rather sure (by looking at the mnemonic used in Chinese UI, which I translated) the "~Sheet > Delete ~Rows" menubar entry the reporter and I saw on Windows is hard-coded, presumably from GenericCommands.xcu, line 7054 [1], and translated with mnemonic on Weblate [2].  One can check a development build with qtz interface to be sure, but I don't have access to one right now.

So it seems different OS/DE use different strings for the menubar?  Or is KDE ignoring the hard-coded mnemonic?

1. https://git.libreoffice.org/core/+/master/officecfg/registry/data/org/openoffice/Office/UI/GenericCommands.xcu#7054
2. https://translations.documentfoundation.org/translate/libo_ui-master/officecfgregistrydataorgopenofficeofficeui/en/?checksum=6236b0b95e37c07e
Comment 8 Heiko Tietze 2022-10-11 08:52:34 UTC
(In reply to Ming Hua from comment #7)
> I'm rather sure ... the "~Sheet > Delete ~Rows" menubar entry ... is hard-coded

Me too :-). It was just a general statement with the idea to remove this/all hard-coded mnemonic.
Comment 9 Sean Gugler 2022-10-15 00:17:39 UTC
(In reply to Heiko Tietze from comment #6)
> you can try to modify via tools >
> customize > menu: find the command at the right-hand list (use the dropdown
> above for the root item), and set the hotkey with a tilde per Modify >
> Rename. For example "Insert ~Rows".

This does work - thank you for the excellent work-around!

I had actually thought of doing this before opening my report, but after trying the "menu rename" interface I (wrongly) concluded that hotkeys could not be assigned this way, for 2 reasons:

1) The prompt in the "Rename Menu" dialog displays the existing name as simply "Delete Rows" without any markup indicating there's anything special about the "R".

2) I experimentally tried inserting an "&" character anyway, which is the native markup symbol used in Windows development, but of course this didn't work. It didn't occur to me to try "~" instead. If the prompt had been shown as "Delete ~Rows" I would have been happily on my way without ever opening a bugzilla ticket.

Maybe I should open a separate ticket that "Customize > Menu > Rename" should display "~" at the appropriate position. I notice that it still displays as "Delete Rows" without any markup, even after I've successfully renamed it "Delete Ro~ws".
Comment 10 Heiko Tietze 2022-11-02 13:34:43 UTC
(In reply to Sean Gugler from comment #9)
> Maybe I should open a separate ticket that "Customize > Menu > Rename"
> should display "~" at the appropriate position. I notice that it still
> displays as "Delete Rows" without any markup, even after I've successfully
> renamed it "Delete Ro~ws".

Yes, good idea.

Back to the topic: We should remove the hard-coded accelerator from commands. But I cannot confirm the issue (it's Delete R~ows) and would prefer to resolve the ticket as invalid. Okay for you?
Comment 11 Alex Pakhotin 2022-11-04 02:05:02 UTC
The previous versions used "D" as a hot-key for the "Delete Rows" command in the menu. The key is unique, makes sense to identify the action ("D"elete), and easy to remember.
Can we please have this back in the next version.
Comment 12 Heiko Tietze 2022-11-08 08:56:11 UTC
(In reply to Alex Pakhotin from comment #11)
> The previous versions used "D" as a hot-key for the "Delete Rows"...

The shortcuts were spread over the modules and we harmonized it for bug 114286 in https://gerrit.libreoffice.org/c/core/+/131699. The R is likely occupied in one module. You can change it locally per tools > customize.
Comment 13 Sean Gugler 2022-12-05 11:52:13 UTC
All right, I've finally taken a look at the git source.

Prior to https://gerrit.libreoffice.org/c/core/+/131699 both "InsertRows" and "DefaultRows" had only "Label" nodes in the XML. Presumably, this meant their hotkeys were computed algorithmically.

After that change, "DefaultRows" now has a "ContextLabel" node that decrees the hotkey is always "R" no matter what other menu items it has as siblings.

Maybe the best general solution is to simply remove that "ContextLabel" node from GenericCommands.xcu and let the conflict-avoidance algorithm operate naturally? My reason for this suggestion is that "InsertRows", in the same file, has no "ContextLabel" node and yet behaves fine.

If this is accepted, consider removing "ContextLabel" from "DeleteColumns" also, for the same reason.
Comment 14 V Stuart Foote 2022-12-20 08:25:38 UTC
*** Bug 152607 has been marked as a duplicate of this bug. ***
Comment 15 V Stuart Foote 2022-12-20 08:32:26 UTC
see https://gerrit.libreoffice.org/c/core/+/131699 for bug 114286

bibisect not required.
Comment 16 BogdanB 2023-09-29 04:57:51 UTC
Removed "bibisect request" based on comment 15.