On Tue, 25 Jun 2024 15:18:29 GMT, Alexey Ivanov <[email protected]> wrote:
>>> In my initial fix, I added the `altProcessor` handler in >>> `SynthLookAndFeel.initialize` with condition check for GTK L&F. Phil has >>> suggested not to check for GTK L&F instead look for some alternate way like >>> mentioned >>> [here](https://github.com/openjdk/jdk/pull/18992#discussion_r1595782003). >>> >>> So, the handler implementation is moved to `SynthRootPaneUI`. >> >> Thus, out of Synth-derived Look and Feels, only GTK needs the feature. Can't >> the listener be installed in `GTKLookAndFeel.initialize`? >> >> At the same time, if you install the listener in `SynthLookAndFeel`, other >> L&F based on Synth could also use this feature, thus your way is more >> flexible. >> >> However, `SynthRootPaneUI` is still part of the base class… I still don't >> understand how it's different from what I'm proposing. >> >> I'd like to avoid keeping a flag whether the listener is installed or not… >> But there could be no other way since not all Synth-based L&Fs enable hiding >> mnemonics. > >> > Requesting the value of RootPane.altPress from UIManager each time >> > postProcessKeyEvent is called is inefficient, so you can store the value >> > in altProcessor when look and feel is installed. >> >> I guess you are pointing out the code in SynthGraphicsUtils.paintText method >> where RootPane.altPress value is retrieved from UIManager each time. Storing >> the value in it's constructor doesn't reflect the correct value and is >> always `false`. > > No, I didn't refer to this particular case. I referred to my suggestion where > you always add `altProcessor` as the keyboard listener and keep track of the > value of the `RootPane.altPress` property. > > Anyway this approach seems to be rejected. > > If RootPane.altPress can be changed dynamically, you can install a > > PropertyChangeListener to UIManager. > > I think it can be changed but is it really required to handle it ? I mean why > does a user change it dynamically ? Not impossible. On Windows, there used to be an option to hide or display mnemonics and focus indicators, and this option could be changed while the app is running. I think there's no option in GNOME, so we shouldn't go out of the way to support dynamic change of the `RootPane.altPress` property. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18992#discussion_r1653037803
