+ 1 :)

Simon's, Samuels and Caolan's refactoring [1] for GTK+3 compliant hiding of accelerators for bug 92630 was necessary, but seems to have complicated things in some areas of the GUI.

IMHO providing a LibreOffice setting for display of accelerators on/off without use of the <alt> key toggle is a needed enhancement. I don't know what would need to happen to make everything behave, the exchange in wont-fix bug 97260 suggests it could be a bit involved.

We also have bug 54169 where behavior of LibreOffice to honor or override Windows system settings is being poked at.

And there are specific GUI issues like bug 92866 regards the Startcenter accelerators, which still aren't right since the accelerators work for GTK+3.

Point is keyboard accelerators are a hot button UX issue, yes LibreOffice should comply with OS/DE behavior, but we should also find methods to allow users to adjust the LibreOffice UI to their preferences.

=-ref-=
[1]  https://gerrit.libreoffice.org/#/c/16883/

Yes this is a major UX issue in not being able to disable this feature and if what Simon mentioned in bug 97260 comment 14 is correct that there isnt a means of disabling this unless you rebuild LO, then i'd suggest that this feature be disabled when compiling against gtk2.

In the GTK+2 case, auto-hiding should not always be disabled; it should be disabled conditionally depending on the setting in the GTK+2 theme file currently in effect for auto-hiding accelerators.

One of the differences between GTK+2 and GTK+3 is that in GTK+2, accelerator auto-hide was controlled by the theme; in GTK+3 this option was removed and the default became auto-hide enabled, with no option to disable it.

The relevant theme variable in the GTK+2 theme file is "gtk-auto-mnemonics = n" where n is 1 to enable auto-hide and 0 to disable it. If behaviour is to be changed for GTK+2, it should respect this setting.

Optionally, the same flag in the theme could be used to enable or disable the feature with GTK+3. Real GTK+3 applications ignore the flag and always auto-hide, but if it was desired, the flag could be supported in LO.

(In reply to Simon Long from comment #4)
> In the GTK+2 case, auto-hiding should not always be disabled; it should be
> disabled conditionally depending on the setting in the GTK+2 theme file
> currently in effect for auto-hiding accelerators.

Yes ideally it shouldnt always be disabled, but with the current state of UX issues that have arisen by following the condition in the gtk+2 theme file, it would be best to disable it when building against gtk2.

> One of the differences between GTK+2 and GTK+3 is that in GTK+2, accelerator
> auto-hide was controlled by the theme; in GTK+3 this option was removed and
> the default became auto-hide enabled, with no option to disable it.
> 
> The relevant theme variable in the GTK+2 theme file is "gtk-auto-mnemonics =
> n" where n is 1 to enable auto-hide and 0 to disable it. If behaviour is to
> be changed for GTK+2, it should respect this setting.

Thanks for the information, though i dont know where to go to change this setting and their isnt a GUI for changing this setting that the novice user would know how to find. It is true that this is a setting in Gtk+2, but that doesnt mean that we must follow it, as there are other gtk+2 apps that dont - e.g. Firefox.

Ultimately with the ability enable and disable this option within LO, package managers and users will be able to easily set this option.

(In reply to Yousuf (Jay) Philips from comment #6)
> (In reply to Simon Long from comment #4)
> > In the GTK+2 case, auto-hiding should not always be disabled; it should be
> > disabled conditionally depending on the setting in the GTK+2 theme file
> > currently in effect for auto-hiding accelerators.
> 
> Yes ideally it shouldnt always be disabled, but with the current state of UX
> issues that have arisen by following the condition in the gtk+2 theme file,
> it would be best to disable it when building against gtk2.

No, it wouldn't - that is a short-term bodge which does the wrong thing.

> > One of the differences between GTK+2 and GTK+3 is that in GTK+2, accelerator
> > auto-hide was controlled by the theme; in GTK+3 this option was removed and
> > the default became auto-hide enabled, with no option to disable it.
> > 
> > The relevant theme variable in the GTK+2 theme file is "gtk-auto-mnemonics =
> > n" where n is 1 to enable auto-hide and 0 to disable it. If behaviour is to
> > be changed for GTK+2, it should respect this setting.
> 
> Thanks for the information, though i dont know where to go to change this
> setting and their isnt a GUI for changing this setting that the novice user
> would know how to find. It is true that this is a setting in Gtk+2, but that
> doesnt mean that we must follow it, as there are other gtk+2 apps that dont
> - e.g. Firefox.

Novice users do not change this setting; theme authors do. All a novice user does is to choose a theme. Theme authors do know where to find this setting, and all applications which use GTK+2 properly obey it - the argument that because some other applications are doing things incorrectly that it is therefore ok for LO to also do things incorrectly is not really sound reasoning.

Implementing code to read this value from the theme and thereupon to enable or disable auto-hiding is a trivial change; it's probably easier than working out how to conditionally build without any of the auto-hide code...

If I get time in the next few weeks I will look at adding the ability to read this value from the theme and to use it to enable or disable auto-hide.

Incidentally - you mention above the ability to compile LO against gtk2 - is that even possible? From what I could see when I was modifying the code, the GTK theming implementation in VCL is purely to emulate GTK+3, not GTK+2. As above, in *all* GTK+3 applications, accelerators are *always* autohidden - the ability to toggle this behaviour is only relevant if you are trying to make VCL look like a GTK+2 application which obeys a GTK+2 theme, and from what I could see, LO doesn't do that at all.

Now I look at the code again, my original patch already implements reading and setting auto-hiding behaviour according to the GTK theme in effect - the lines:

+    g_object_get (gtks, "gtk-auto-mnemonics", &val, NULL);
+    if (val) pSVData->maNWFData.mbAutoAccel = true;
+    else pSVData->maNWFData.mbAutoAccel = false;
+    g_object_get (gtks, "gtk-enable-mnemonics", &val, NULL);
+    if (val) pSVData->maNWFData.mbEnableAccel = true;

in salnativewidgets-gtk.cxx do just this.

Have you actually tried setting the value of gtk-auto-mnemonics in your theme to 0?

If you add the lines 

gtk-enable-mnemonics = 1
gtk-auto-mnemonics = 0

to the start of your gtkrc file, accelerator underscores should be shown all the time.

To give more detail, from https://developer.gnome.org/gtk3/stable/GtkSettings.html :

"On the X window system, this sharing is realized by an XSettings manager that is usually part of the desktop environment, along with utilities that let the user change these settings. In the absence of an Xsettings manager, GTK+ reads default values for settings from settings.ini files in /etc/gtk-3.0, $XDG_CONFIG_DIRS/gtk-3.0 and $XDG_CONFIG_HOME/gtk-3.0. These files must be valid key files (see GKeyFile), and have a section called Settings. Themes can also provide default values for settings by installing a settings.ini file next to their gtk.css file."

On my Raspberry Pi running LXDE, these are managed by the lxsession session manager. If I go into its configuration file (~/.config/lxsession/LXDE-pi/desktop.conf), there is a section labelled [GTK] which includes the two parameters

iGtk/AutoMnemonics=1
iGtk/EnableMnemonics=1

If I change the first line to

iGtk/AutoMnemonics=0

and reboot, the accelerators are always shown in LO.

This is the correct way to enable / disable showing of accelerators - to change the setting which GTK uses in the theme. If you are running a different session manager, or indeed not running one at all, there will be alternative methods for setting these variables. But I've checked the top of tree in the repo, and the mechanics to turn this feature on and off were pulled from my patch and are therefore already implemented in LO - there is no need for anything to be changed.

If someone can confirm the above works in the latest builds of LO, I suggest this issue should be marked as resolved - as far as I can see, the standard GTK mechanism for enabling and disabling auto-hiding of accelerators is already implemented and working.

Hm I think an application-level option to override the system default, is still a valid enhancement request.

Although I wonder what exactly the "major UX issue" is, that you are speaking of, Jay.

(In reply to Samuel Mehrbrodt (CIB) from comment #11)
> Hm I think an application-level option to override the system default, is
> still a valid enhancement request.

But that isn't how GTK applications work. The whole point of GTK theming is that *all* applications built on GTK share the same look and feel, which comes from the theme. GTK applications do not have app-specific theme overrides, as why would you want to override the theme behaviour in only one application, rather than in all of them?

If you want to change the look and feel of a GTK application, you change it consistently in all of the applications on your system by changing the theme; you don't do it on a per-application basis. To do so makes theming pointless.

LibreOffice is a GTK application? I thought it was cross-platform ;)

I don't think it unreasonable to provide simple control in GUI (and any needed linkages per OS and DE) to set the attribute from within LibreOffice.  Provide application controls to either ignore system setting--selectively--for LibreOffice, or to actually adjust the system setting from within LibreOffice if needed. 

We also have to be mindful of the impact on accessibility and how we implement assistive technology tools, i.e. exposure of accessible events and associated tooltips (default, and extended that are a part of the non-visual UI). In this regard we can't hide the visible accelerators if it also disrupts the AT. 

There is a certain "idealism" in Simon's position that the theme alone should control the LibreOffice UI, and that it should be GTK+3 compliant. A user needing exposed accelerators would select a theme that always showed them. OK, and enhancements of bug 92630 provided a framework within LibreOffice for just that--but doesn't that only cover Linux GTK based DEs?

Seems our reality cross platform is that we still have to provide the controls for the other OS and non-GTK DE.  In the case of Windows OS, that means tweaking the HKEY_CURRENT_USER\Control Panel\Accessibility\Keyboard Preference (remaining issue of bug 51469). OS X passes on the whole issue, and no idea where qt/KDE based GUI falls.

(In reply to V Stuart Foote from comment #13)
> I don't think it unreasonable to provide simple control in GUI (and any
> needed linkages per OS and DE) to set the attribute from within LibreOffice.

Same here..

(In reply to V Stuart Foote from comment #13)
> (remaining issue of bug 51469).

s/bug 51469/bug 54169

(In reply to V Stuart Foote from comment #13)
> LibreOffice is a GTK application? I thought it was cross-platform ;)

Surely the sole point of the GTK compatibility module is to make LO appear as if it is as close to a GTK application as possible? The auto-accelerator behaviour under discussion is *only* exhibited if the GTK module is enabled. Why would someone make the choice to enable the GTK compatibility mode if not in order to align the look and feel of LO with the rest of their GTK applications?

> There is a certain "idealism" in Simon's position that the theme alone
> should control the LibreOffice UI, and that it should be GTK+3 compliant. A
> user needing exposed accelerators would select a theme that always showed
> them. OK, and enhancements of bug 92630 provided a framework within
> LibreOffice for just that--but doesn't that only cover Linux GTK based DEs?

As above - the issue of hidden accelerators only (as far as I am aware) occurs if the GTK compatibility module is selected. Yes, I am an idealist in as much as I very much believe that if the GTK compatibility module is selected, then LO should follow as closely as possible the GTK theme in effect. If the GTK module is selected and there are additional options which need to be selected within LO itself to enable LO to conform to the theme, that is simply going to add confusion to anyone used to GTK, within which applications do not have theme overrides.

My position is that if the GTK compatibility module is not used, then provide whatever UI customisations you see fit by whatever means is appropriate - that makes perfect sense. But if a user has deliberately made the decision to enable the setting which makes LO follow the GTK theme, then additional options which cause LO to unnecessarily deviate from the theme are nothing but a potential source of confusion. (And speaking as a UI designer, deviations from consistent behaviour in Linux desktop apps are the absolute bane of my life, and I spend half my time trying to fix them...)

TLDR; the GTK VCL module should follow theme without deviation; that's how GTK works and what people accustomed to GTK environments would expect. If the GTK module is not used, then offer whatever application-specific customisation is desired. But please don't combine customisation with the GTK compatibility module!

(In reply to Simon Long from comment #7)
> (In reply to Yousuf (Jay) Philips from comment #6)
> > Yes ideally it shouldnt always be disabled, but with the current state of UX
> > issues that have arisen by following the condition in the gtk+2 theme file,
> > it would be best to disable it when building against gtk2.
> 
> No, it wouldn't - that is a short-term bodge which does the wrong thing.

Not really a short-term dodge when it has been LO's behaviour since its establishment. The point is to be conservative with this change (limit it to be enabled only in gtk3) until it is fully matured.

> > Thanks for the information, though i dont know where to go to change this
> > setting and their isnt a GUI for changing this setting that the novice user
> > would know how to find. It is true that this is a setting in Gtk+2, but that
> > doesnt mean that we must follow it, as there are other gtk+2 apps that dont
> > - e.g. Firefox.
> 
> Novice users do not change this setting; theme authors do. All a novice user
> does is to choose a theme. Theme authors do know where to find this setting,

When we move from an interface where we didnt support auto-accelerator to one where we do, users will first assume that its either a bug (like i did in bug 97586, as well as others) or its a feature that has been implemented in LO, rather than one that is being implemented based on a theme-level or OS-level property. Users pick a theme based on what it looks like and not on the value of the auto-accelerator property, so to say that users wouldnt want to change this feature is inaccurate, as i'm unable to changing this feature in a easy way and can imagine what a novice user will go through.

Some gtk+2 desktop environments (e.g. Xfce, Mate) provide users with the ability to tweak their theme in a GUI interface, like being able to modify the controls, icons, window borders, etc. They dont give the option to change the accelerator property, so users who choose a theme which happens to have it enabled dont have the ability to disable it there without jumping through hoops.

> and all applications which use GTK+2 properly obey it - the argument that
> because some other applications are doing things incorrectly that it is
> therefore ok for LO to also do things incorrectly is not really sound
> reasoning.

The point I was attempting to get across is that we dont have to adhere to every GTK+2 property if we believe it has a negative affect to our users user experience. We havent been adhering to this property since LO's establishment, so we dont have to adhere to it now just because.

> Implementing code to read this value from the theme and thereupon to enable
> or disable auto-hiding is a trivial change; it's probably easier than
> working out how to conditionally build without any of the auto-hide code...

Yes it is fine that we have the code that does this reading of the value, i am saying that we should include an IF statement after the reading of this value that if the user has left the option to show or hide auto-accelerator in the options dialog to 'Automatic' (likely stored in registrymodifications.xcu as a NULL value) and LO was built against gtk2 (built with the argument --disable-gtk3) that auto-accelerator should be disable by default unless a user/packager enables it. Below is an outline of how it would be written in php:

auto_accelerator = value read from theme or OS;
auto_accelerator_options_level = value read from registrymodifications.xcu;

if ( auto_accelerator_options_level != NULL ) {
   auto_accelerator = auto_accelerator_options_level;
} else {
   if ( gtk2_build ) {
      auto_accelerator = 0;
   }
}

> If I get time in the next few weeks I will look at adding the ability to
> read this value from the theme and to use it to enable or disable auto-hide.

I assume as this gtk2 theming attribute hasnt been implemented yet that i'm being affected by a gtk3 theme on my gtk2 desktop environment.

> Incidentally - you mention above the ability to compile LO against gtk2 - is
> that even possible?

The daily builds that TDF compiles are against gtk2 and i believe the ones available on the libreoffice.org download page are also against gtk2.

(In reply to Samuel Mehrbrodt (CIB) from comment #11)
> Although I wonder what exactly the "major UX issue" is, that you are
> speaking of, Jay.

As stated, "this is a major UX issue in not being able to disable this feature". Meaning not being able to disable this incomplete feature at the application level and users having to go to the theme or OS level to do so.

(In reply to Simon Long from comment #12)
> But that isn't how GTK applications work. The whole point of GTK theming is
> that *all* applications built on GTK share the same look and feel, which
> comes from the theme. GTK applications do not have app-specific theme
> overrides, as why would you want to override the theme behaviour in only one
> application, rather than in all of them?

LO has app-specific theme and OS overrides options like whether to show icons in menus, what font to use for the interface, and what option to use for middle mouse-click, so this wouldnt be something new. Well i would want to overwrite this theme or OS behaviour exclusively for LO, as it is the only application that i use accelerators in (never even noticed that accelerators werent there in any other app i use, but noticed it immediately when they disappeared in LO) and the auto-accelerator feature is broken in various places (bug 99324).

(In reply to Yousuf (Jay) Philips from comment #17)
> > Novice users do not change this setting; theme authors do. All a novice user
> > does is to choose a theme. Theme authors do know where to find this setting,
> 
> When we move from an interface where we didnt support auto-accelerator to
> one where we do, users will first assume that its either a bug (like i did
> in bug 97586, as well as others) or its a feature that has been implemented
> in LO, rather than one that is being implemented based on a theme-level or
> OS-level property. 

Yousuf, you are looking at this issue solely as a LO user. You are not looking at it as a user of a GTK environment. As I have asked above, what is the purpose of GTK compatibility in LO unless to make LO match look and feel of the other applications in a GTK environment in which it is being used? I cannot think of one.

Your point seems to assume that someone is using LO as the only GTK application on their system - as otherwise surely they would notice that they have the auto-accelerator feature in all their other applications? And if someone did notice that they had auto-accelerators enabled in multiple applications and wanted to turn it off, by your argument, every GTK application would need to individually offer them that as an option - as I have said before, that is not how GTK applications work.

> Users pick a theme based on what it looks like and not on
> the value of the auto-accelerator property, so to say that users wouldnt
> want to change this feature is inaccurate, as i'm unable to changing this
> feature in a easy way and can imagine what a novice user will go through.

You are trying to dissociate a GTK theme behaviour (auto-accelerators) from GTK theme appearance, but the two are intrinsically linked; that's what a theme is for. If a user picks a theme with auto-accelerators, they will get auto-accelerator behaviour in all GTK applications.

> Some gtk+2 desktop environments (e.g. Xfce, Mate) provide users with the
> ability to tweak their theme in a GUI interface, like being able to modify
> the controls, icons, window borders, etc. They dont give the option to
> change the accelerator property, so users who choose a theme which happens
> to have it enabled dont have the ability to disable it there without jumping
> through hoops.

But what does it benefit a user to have a way to disable the feature in one single application? If they don't like it, they will surely want to disable it in all applications - providing a means to ignore theme in LO will only make them look for a similar mechanism in other applications, and they won't find one - because it shouldn't be set in applications.

Your point about some environments not allowing this parameter to be changed in their user-friendly theme editors is not an argument for adding an override to LO; it is an argument for adding the ability to globally set the parameter to the theme editors.

> The point I was attempting to get across is that we dont have to adhere to
> every GTK+2 property if we believe it has a negative affect to our users
> user experience. We havent been adhering to this property since LO's
> establishment, so we dont have to adhere to it now just because.

This was a bug in LO's GTK emulation which has been fixed - that's why we didn't adhere to it since LO's establishment; LO was doing the wrong thing before this fix. LO is now doing the right thing. The argument that we have been doing the wrong thing for so long that we should never fix it is not a valid one. We should absolutely attempt to adhere to every GTK property if we are offering GTK compatibility - as otherwise, what's the point in offering it?

Simon, it seems that we arent going to see eye to eye on this, so there isnt a need to go back and forth more about it, but if your solution to this is to tell LO users that have auto-accelerator enabled in their theme and want to disable it, to modify a configuration file or speak with their desktop environment to have them modify their theme editors so that users can set it there, i'm sorry but that is not a valid solution that any user will accept.

(In reply to Yousuf (Jay) Philips from comment #19)
> Simon, it seems that we arent going to see eye to eye on this, so there isnt
> a need to go back and forth more about it, but if your solution to this is
> to tell LO users that have auto-accelerator enabled in their theme and want
> to disable it, to modify a configuration file or speak with their desktop
> environment to have them modify their theme editors so that users can set it
> there, i'm sorry but that is not a valid solution that any user will accept.

In your opinion. It is the solution which is used in GTK - you may not personally like it, but as I have now stated repeatedly, it is how GTK works; it is how every other application written on a native version of GTK works. 

If LO natively supported GTK, we wouldn't even be having this discussion - this would be how LO worked, and that would be it - auto-accelerator behaviour would follow theme, full stop.

Your argument is for a half-finished implementation of GTK emulation which happens to fit your personal needs. It is not the right way to implement GTK emulation for anyone who is used to using a GTK environment and using other GTK applications within it, which is why it was reported as a bug in LO over 4 years ago - see https://bugs.documentfoundation.org/show_bug.cgi?id=54169 for details.

Reverting this behaviour would be to cause a regression on this bug.

Yeah, I'm with Simon with this. The gtk integration should just do what the theme does and not fight against it. Ideally we wouldn't even have to "follow the theme" if we were fully native, and its likely we'll go more down the road of using true native widgetry, e.g. right now the gtk3 menus and menubar for example are "true" native menubars and their behaviour is mostly out of our control now.

If we insist on following the system theme, can we give the users the option to change the system theme settings? If we did this in the installer or main application, I think it would be a fair compromise. 

I don't care if it's system level or application level, but we should the give user the option to continue to use the old behavior.

Luke: I don't think anybody will veto code that adds the option to overwrite the setting [though I think we already have way too many options] or anything like that here.

But as you can see above, the developers involved in the gtk3 support don't agree with the approach, so the only way to persuade them is to provide the code yourself, or to find somebody who will do the coding for you.  It's unlikely that the developers will spend time hacking on something they don't agree with :-)

(In reply to Simon Long from comment #20)
> Reverting this behaviour would be to cause a regression on this bug.

As stated multiple times, i'm not suggesting this be reverted, i'm suggesting an app-level overwrite.

Ubuntu and OS X disable icons in the menus and we provide an app-level overwrite to change this. Linux desktop environments uses middle-click to paste text and we provide an app-level overwrite to change this. LibreOffice should support Gtk2 and Gtk3 for auto-accelerator and we should provide an app-level overwrite.

(In reply to V Stuart Foote from comment #2)

> Point is keyboard accelerators are a hot button UX issue, yes LibreOffice
> should comply with OS/DE behavior, but we should also find methods to allow
> users to adjust the LibreOffice UI to their preferences.

Indeed.
For power users in LibreOffice the situation may be that they sort of accept GTK+3 behaviour for other applications but are annoyed by it in LibreOffice. It simply forces to hit extra keys.
So I don't see the problem with a LibreOffice setting at all.

*** Bug 100278 has been marked as a duplicate of this bug. ***

at this point our GTK UI is mostly using native gtk widgets so the autoaccelerator is mostly a native gtk feature (instead of us emulating gtk) and out of our direct control