RFR: 8340852: ScrollPane should not consume navigation keys when it doesn't have direct focus

[Andy Goryachev]

On Thu, 3 Oct 2024 19:40:43 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/ScrollPaneBehavior.java line 88:
>> 
>>> 86: 
>>> 87:             new InputMap.KeyMapping(new KeyBinding(HOME), e -> verticalHome(), this::isNotFocused),
>>> 88:             new InputMap.KeyMapping(new KeyBinding(END), e -> verticalEnd(), this::isNotFocused),
>> 
>> minor: this change creates a bunch of lambdas
>> suggestion: declare 
>> 
>>         Predicate<KeyEvent> isNotFocused = (ev) -> {
>>             return !getNode().isFocused();
>>         };
>> 
>> and pass that to each key mapping instead
>
> It's an interesting suggestion, but it is not needed.  `javac` will already deduplicate these.
> 
> You can even verify that this is the case.  Use `javap` to decompile the class file with `javap -c <classname>`.  In there, `invokedynamic` is used to represent the lambda's.  It looks like this for example:
> 
>      120: invokedynamic #54,  0             // InvokeDynamic #1:test:(Lcom/sun/javafx/scene/control/behavior/ScrollPaneBehavior;)Ljava/util/function/Predicate;
> 
> Later on, you'll see another:
> 
>      152: invokedynamic #54,  0             // InvokeDynamic #1:test:(Lcom/sun/javafx/scene/control/behavior/ScrollPaneBehavior;)Ljava/util/function/Predicate;
> 
> What you can see here is that the same constant (# 54) is used to reference the method. So, there's no need to help the compiler here.

hmmm... when I set up a breakpoint in Eclipse in KeyMapping:785 (after it hits the ScrollPaneBehavior<init>) the value for `interceptor` changes


interceptor= 0x0000007001340d50  (id=208)	
interceptor= 0x00000070013411c0  (id=212)	
interceptor= 0x0000007001341630  (id=216)	


I think it's still creates a different lambda object.  (I recall testing this assumption with some unit test a while back).

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1582#discussion_r1786773525