[External] : Re: [Request for Comments] Behavior / InputMap
John Hendrikx
john.hendrikx at gmail.com
Sat Oct 14 17:19:49 UTC 2023
On 12/10/2023 21:56, Andy Goryachev wrote:
>
> Filters: are we talking about key bindings or event handlers? With
> the key bindings, the user mappings take precedence over those
> registered through behavior. There is no provision for adjusting
> priority of the event handlers – that’s the FX reality, we don’t get
> to rearrange event handlers within the node. That’s why I mentioned
> event filters added _to the control_. I am not sure why you talk
> about HBoxes – adding a filter on the control enables the user to
> intercept the event prior to skin/behavior.
>
On simple Controls yes, filters can be used there for this purpose, even
though that's not their purpose. It works because a Control (usually)
is a leaf node. It breaks down however when you want to change behavior
of the View controls which have deeper control layers. You may want to
override something defined for ListView, but only if say a focused
editable control isn't consuming that event for a different purpose. A
filter won't be able to do this.
> So yes, this proposal does not address the event handlers (sorry for
> confusing key bindings with event handlers). Unless we add
> addListenerBefore() API, we’d need to use event filters – but this is
> out of scope for this proposal.
>
> I do agree with you that we should keep the concrete Behaviors private
> for now. In any case, public behaviors are outside of scope for this
> proposal.
>
I see BehaviorBase moving to a public package though, and it is not
package private, is that intended then?
> One thing you mentioned several times is a “truly good design”. Let’s
> hear it! Could you give us an outline, at the public API level at
> least, of such a design?
>
Alright; these things take a lot of time, but I've taken a few hours to
think about it today. First, a lot of things just triggered me with the
current proposal;
- The mutable maps, immutability would allow making these static, saving
many objects; when I have some time I can take a look at how many are in
memory when a decent sized FX application is running; as even Labels are
Controls, and Labels are the base class for any Cell, this number might
be larger than expected and potentially could allow for some significant
memory savings; making it public as-is closes that road down forever.
Immutability does not mean things can't be changed, it just requires a
slightly different mindset (ie. you combine a standard InputMap with a
custom InputMap to form a new InputMap in which a binding is
changed/overriden/disabled); this was also proposed on JDK-8091189
- The Control back reference; I firmly believe this is unnecessary, and
also very undesirable. Events already contain this reference, its
superflous and requires a new instance for an (otherwise) similar
instance; This is even done already in places, exactly to avoid having
to create more instances (see #getNode in FocusTraversalInputMap for
example, effectively allowing that class to be static while providing
the focus traversal behavior). This was also raised on JDK-8091189
- Not designing this using interfaces (also raised on JDK-8091189).
With the addition of default methods, we should favor composable designs
instead of being forced to inherit from a base class in order to provide
a custom implementation. Sure, you can still provide a default
implementation, but public methods should be referring to the interface
so it can be a completely different implementation. Interfaces prevent
using package private shortcuts where privileged classes can provide a
functionality that users cannot.
- The public BehaviorBase and the new public behaviors for many
controls; I'm not convinced behaviors (if we can't define exactly what
their purpose is supposed to be vs the Control and Skin) are ready to
become more than just an implementation detail.
As for high level design, it of course depends on what the goal here
is. The issues linked in the proposal all call out for a public
behavior API, but your proposal narrows the scope rightfully down to
influencing default reactions to (key?) events only. Making behaviors
public as they are now, and without a clear definition of what they are,
seems definitely premature. I think they first need to be re-evaluated
to see if they are still a good design at all (after all they're 10+
years old), then rewritten internally (if the ideas behind them
changed), and only then made public.
In your proposal I think the Summary and Motivation are quite good at
outlining a problem to be solved. I'd like to rephrase that as a goal:
"To allow developers to remap, override or disable the standard behavior
of JavaFX controls (note: behavior here is used in the general sense)".
I think there is no need to mention InputMap, Behaviors or key bindings
here, those are merely possible means to achieve the goal.
I'd then first take a look how this could be achieved with current
JavaFX, and where users are running into a wall.
Most obviously, the way to make controls do something you want is by
using event handlers. Even Behaviors use these internally, in which
we'll later see lies a bit of the problem.
# Expectations
Just like when a developer sets a Control property directly to a
specific value, when the developers adds an event handler or listener,
he/she can rightfully expect that FX respects this and does not get in
the way. For the property case, CSS will not override such a property,
for listeners, all listeners receive any callbacks, and for events, the
user registered handlers should take priority over internal handlers
(unlike listeners, event handlers can consume and act upon events before
they reach the user handler, hence order plays an important role here).
CSS, Skins and Behaviors sharing the same properties, listeners and
event handlers with application developers has always been a bit of a
balancing act; CSS has an elaborate system to respect user property
changes, and keeps track of these; Skins for the most part manage to
stay out of the application developer's way, mostly because they
primarily use listeners which inherently don't block listeners added by
the application developer. They also rarely override properties
outright that are also modifiable by the developer.
With Behaviors the situation is very different. Event handlers added by
behaviors will consume events, effectively acting upon them before the
application developer can (you may still see such events as "consumed",
but they will not bubble up further). On top of that is the fact that
EventHandlers are far more complicated than plain listeners or
properties. For example, a KeyEvent.KEY_PRESSED handler is called
before a KeyEvent.KEY_ANY handler; attempting to override behavior in a
KeyEvent.KEY_ANY handler is therefore impossible when the behavior to
override is using a more specific event type. Consumption of an event
only blocks capturing/bubbling of the event; other event handlers at the
same level do still see such events, but they're marked "consumed"; most
event handlers should therefore probably start with a `if
(e.isConsumed()) return` line, but often don't (users often don't do
this because they expect their handlers to be the only ones present, or
to be called first, even though for Controls with Behaviors this is not
actually true).
# Problems
Some of the problems you can expect to see when you want to act upon an
event that has a "default" behavior (versus ones that don't):
- Adding a more generic event handler than the one used internally will
result in the events you are interested in being consumed already
- Adding the exact same event handler as one used internally AFTER the
control was shown (or re-adding it in response to something) will result
in events you are interested in being consumed already, or more
generally, an event handler works differently whether they were added
before or after the control was shown...
- Events for which there exist a default behavior are in some cases
consumed even if the behavior could not be performed (navigation)
In all the above cases, for events WITHOUT default behavior, such a user
installed handler works exactly as expected. IMHO there really should be
no difference for the user whether there is a default behavior or not.
# Causes
Almost all of these problems are caused by the fact that JavaFX's
internal handlers share the same lists/mechanisms as application
developers to react to events; these internal handlers are mixed up with
event handlers from application developers; often the internal ones run
first, but it is very unpredictable:
- Is your event handler more generic than an internal handler? You
always run last
- Is your event handler more specific than an internal handler? You
always run first
- Is your event handler the exact same event type as an internal
handler... then:
- Did you add handlers before the control was shown / skin was
created? Yours run first (subject to change no doubt, we don't guarantee
this)
- Did you add handlers after the control was shown? Yours run last
(no guarantees)
- Did you add handlers after the control was shown, but then its
skin was replaced? Your event handlers that used to run last now run
first... (undocumented)
An innocent change like listening for KeyEvent.ANY vs
KeyEvent.KEY_PRESSED can have big repercussions.
# How to reach the goal?
There are many ways to reach the above stated goal. Opening up some
internal structures that are used by the internal event handlers is one
way, but it effectively creates a 2nd mechanism to do the same thing. I
can change the internal event handler's internal structures to do X, or
I can create an event handler that does X. For some of these, both
options work, and for others, only this new mechanism works. Not only
is this mostly compensating for a flaw in the event handling system, but
it also means that you need to be aware of which events need special
treatment. It's even possible that some events require no special
treatment now, but may in the future, or may need it if the platform
changes certain defaults. In other words, this new mechanism would
effectively need to be used in all cases or you risk your solution
(using standard event handlers) breaking in the future (or JavaFX would
have to freeze input maps and never change them again -- that's already
sort of the case, but it is good to be aware of that).
# Alternative solution
I would look into seeing if the flaws in the event handling system can
be resolved, so that this mechanism that is already available, and that
users already know becomes powerful enough to cater to the stated goal.
Note that this doesn't preclude opening up behaviors later, but it does
take away one of the fundamental reasons to do so, perhaps allowing for
quite a different way of exposing these to users as the primary driver
need no longer be focused on remapping bindings. Perhaps the primary
driver can then be how to design behaviors in such a way that they can
be re-used and easily subclassed; perhaps behaviors are no longer needed
at all, and they can remain an internal implementation detail, or
perhaps they can be part of skins or controls.
I see a few problems that should be addressed if we want to be able to
reach the goal with better event handlers:
1) Internal event handlers should NOT use the same mechanism as user
event handlers; effectively, user event handlers (of any event type,
even more general ones) run first, as if no internal event handlers
existed at all. This is already the case depending on the timing of
when the user added the handlers; the timing is unpredictable (as stated
above) and so I think we have enough leeway to change this, and enough
reason to tighten up the specification here.
2) All internal event handlers run AFTER user ones (per EventTarget),
regardless of when they were added. A separate list can be used, or the
event type system could support this with some kind of internal flag.
3) Internal event handlers can be skipped completely if the user marks
an event as such. This is different from consuming the event;
effectively, the event is not consumed (and can bubble up to other event
targets) but internal event handlers are not allowed to act upon it.
4) All actions triggered by behaviors should be available as public
methods (nested or direct) on their respective controls, allowing the
user to call these as well.
The above changes should be enough to support the stated goal: "To allow
developers to remap, override or disable the standard behavior of JavaFX
controls (note: behavior here is used in the general sense)"
To override a standard behavior: install event handler (which will run
first), react to an event, call a public method triggering the DIFFERENT
behavior, consume the event
To disable a standard behavior: install event handler, react to an
event, mark it as "not for internal use", effectively disallowing the
internal handlers from acting on it
To remap a standard behavior: combine the above two solutions
# New API needed
A flag on `Event` that can be set/cleared whenever the user wants. The
flag effectively communicates that the given event is not to be
processed by the "hidden" internal event handlers added by JavaFX. It
could be called "noDefault" or "skipDefaultBehavior".
Depending on the internal changes needed to separate user event handlers
from internal ones, EventType may also need a small change. For
example, if the separation is implemented by introducing more event
types, a convenient `EventType#toInternal` method could be added to
convert a regular event type to an internal one that is always processed
after standard ones. It's possible such a method does not need to be
public (but could be if users desire the old unpredictable behavior of
mixing user and internal event handlers).
# Alternative alternative solution
Part of the problem can also be solved by disallowing internal handlers
to listen on the most specific EventType (ie, don't listen to
KeyEvent.KEY_PRESSED, but instead listen to KeyEvent.ANY). This way a
user can be the first to handle the event in all cases by using a more
specific type (KeyEvent.KEY_PRESSED) or the last in all cases by using a
less specific type (InputEvent.ANY). This leaves much to be desired,
and doesn't solve all of the above outlined problems, but I mention it
to show that there is quite a lot possible here already by tweaking the
event handling system.
--John
> Thank you
>
> -andy
>
> *From: *John Hendrikx <john.hendrikx at gmail.com>
> *Date: *Thursday, October 12, 2023 at 01:33
> *To: *Andy Goryachev <andy.goryachev at oracle.com>,
> openjfx-dev at openjdk.org <openjfx-dev at openjdk.org>
> *Subject: *[External] : Re: [Request for Comments] Behavior / InputMap
>
> On 11/10/2023 19:44, Andy Goryachev wrote:
>
> Dear John:
>
> Seems like addEventFilter() was specifically designed to intercept
> events before any other handlers, so I see no problem there.
>
> This is a misunderstanding of what filters are for. They're called in
> the capturing phase and they can prevent an event from reaching its
> intended target, but you want it to reach the intended target FIRST,
> as you still want to give the target the chance to be the first to act
> upon the event. For example, let's say I want to attach a function to
> the SPACE key in some specialized HBox, it should only do something
> when something closer to the target doesn't need it first (like a
> nested HBox of the same type, or an active TextField that uses SPACE
> for input). Now if HBox had some FX default event handler that
> consumed SPACE, I have no options to override SPACE anymore; the
> filter is not a good spot, as its too early, and the handler is not a
> good spot because Behavior handlers were added before mine was.
>
>
>
> I somewhat disagree about the purpose of the key mapping system –
> the proposed solution solves two existing issues (the
> skin/behavior mappings and the user mappings) in one neat package.
> Every other instrument such as addEventHandler/Filter is still there.
>
> I'm saying that the need for this would be almost non-existent if user
> event handlers weren't considered less important than FX ones. You
> have to be careful that there aren't two ways of doing things here:
>
> If the event you wish to give an alternative purpose to is unused by
> FX, you can use an event handler; otherwise you must disable it (so
> you can use an event handler!) or remap it (using an alternative
> system). Note that if FX at some point decides to "claim" another
> mapping, that would be a breaking change as some user event handlers
> may cease to function.
>
> This is why I think the input mapping system should stay hidden; its
> an implementation detail of the Event handlers added by FX so they
> don't need to write long if/else/switch chains, and so they can more
> easily switch out mappings depending on state. Opening up the input
> map effectively is being able to influence those FX added event
> handlers to do something different, while there is a perfectly good
> way of doing that already: add your own event handler (with higher
> priority).
>
> And, if we look at the three bullet points
>
> - Ensure user event handlers have priority over behavior/inputmap
> added ones
> - Ensure all behavior actions are available as methods on controls
> - Ensure that if a key is handled by the control, that it is ONLY
> consumed when it actually triggers an action (navigation keys get
> consumed regardless, even if no focus change results, that's wrong).
>
> I absolutely agree, and in fact the first three are indeed a part
> of the proposal. Well, the 3^rd one might unfortunately be a
> subject of backward compatibility limitation since one of the
> requirements was no behavior change w.r.t. the earlier versions.
> We can always change the behavior if we have a completing reason
> and go through the usual process, nothing in the proposal
> precludes that.
>
> I don't see how your proposal addresses the first point.
>
> I've been reading the comments attached to
> https://bugs.openjdk.org/browse/JDK-8091189 and it has a lot of good
> information, and raises many good points (immutable input maps, keep
> input maps/behaviors as implementation details, use interfaces instead
> of base classes, what about controls that have no Skin, even the point
> I made about having the Control be in charge of registering the event
> handlers instead of letting InputMap do it requiring a Control
> field...). There are several patches by Jonathan Giles, and there is
> even a library created by the author of ReactFX that allows for
> replacing key bindings with a much nicer API already (in so far that
> is possible without having inside FX support).
>
> The general tone of the comments seems to be that Behaviors should be
> kept as implementation details -- they're not well defined (what is a
> Behavior, what should be in the Behavior, what should be in the Skin
> and what should be in the Control) and until that is absolutely clear,
> exposing any of that as API is premature.
>
> Making the behaviors completely independent with setBehavior() and
> FXML, as you said, might be a future effort, perhaps we could
> attempt opening up certain controls at some point. On one hand, I
> am for increasing the extensibility of FX, on the other hand the
> same argument can be made against it (as in solidifying a
> particular way of constructing skins and behaviors), but I feel
> it’s a separate issue that is independent of this proposal.
>
> I'm starting to lean towards keeping all of this as implementation
> details, at least until the current implementation is much cleaner
> than it is currently (the way InputMap and Behaviors currently are set
> up is more pragmatic than truly a good design), and just address the
> main issue: JavaFX stealing events that users want to override, note
> that I say events, key bindings are only part of it.
>
> --John
>
> -andy
>
> *From: *openjfx-dev <openjfx-dev-retn at openjdk.org>
> <mailto:openjfx-dev-retn at openjdk.org> on behalf of John Hendrikx
> <john.hendrikx at gmail.com> <mailto:john.hendrikx at gmail.com>
> *Date: *Wednesday, October 11, 2023 at 01:04
> *To: *openjfx-dev at openjdk.org <openjfx-dev at openjdk.org>
> <mailto:openjfx-dev at openjdk.org>
> *Subject: *Re: [Request for Comments] Behavior / InputMap
>
> I'm sorry, but that providing an arbitrary key mapping system
> seems completely out of scope and not something that JavaFX should
> concern itself with. It's much too high level, when the key
> mappings involved should only be for actions that the control can
> provide on its own.
>
> I think the problem we should be solving is that JavaFX control
> behaviors shouldn't be first in line when it comes to consuming
> events (which currently is only the case due to event handlers
> being added at the earliest possible opportunity, and event
> handlers being called in order). If something as trivial as:
>
> control.addEventHandler(KeyEvent.KEY_PRESSED, e -> {
> if (e.getCode() == KeyCode.LEFT) {
> e.consume(); // stop default behavior
> }
> });
>
> ... actually worked, then there is much less need to
> redefine/disable behavior key mappings, and no need for a
> secondary system that deals with mappings (the first system, event
> handlers, can simply be used for this). If user event handlers
> had priority over behavior ones, then everything you want can be
> achieved with the above, including:
>
> - Stopping default behavior
> - Triggering different behavior (just call something on control,
> of course, make sure all behavior actions are available on the
> control in the first place)
> - Remapping (a combination of the above two)
> - Adding an alternative key for the same behavior
>
> A system to remap keys can then be left squarely in the realm of
> user space, and much nicer solutions can be build by users than
> whatever JavaFX will provide out of the box.
>
> Changes to the Behavior system can then focus on replacing
> complete behaviors (including their input map) and being able to
> use these by default for a certain subset of controls (like
> -fx-skin provide in CSS), as this is something users currently
> can't do.
>
> So in short, what I think this should be about is:
>
> - Ensure user event handlers have priority over behavior/inputmap
> added ones
> - Ensure all behavior actions are available as methods on controls
> - Ensure that if a key is handled by the control, that it is ONLY
> consumed when it actually triggers an action (navigation keys get
> consumed regardless, even if no focus change results, that's wrong).
>
> Future:
>
> - Make behaviors public and allow Behaviors to be replaced with
> -fx-behavior type CSS syntax / control.setBehavior calls
>
> --John
>
> The focus should be on being able to modify standard behavior of
> controls (arrow-left, enter, ctrl-shift-right, etc.), specifically
> also to be able to disable these when undesired, and, on top of
> that, that they bubble up when NOT used even when they are
> configured (focus navigation keys currently are always consumed,
> whether they actually do something or not -- that's a big issue).
> The other focus should be on providing an alternative behavior (or
> at least mappings) for all controls of a certain type -- I don't
> see the need for adding a mapping to a specific control, that's
> already covered with event handlers; the problem is mostly that
> behaviors currently steal certain events before the user can get
> at them.
>
> Custom behaviors can then be constructed that provide more things
> that may need mapping. I'd expect those however to be limited in
> scope to what the control offers, certainly not an arbitrary
> key/action mapping system (that wouldn't even work, as most of
> these would be in the scope of several controls or be global).
> This kind of functionality is much better provided by event
> handlers at the correct level for a group of controls, and I
> wouldn't expect to find such an eloborate system incorporated in
> behaviors.
>
> In fact, thinking about all of this a bit more,
>
> On 10/10/2023 19:54, Andy Goryachev wrote:
>
> Re-sending with a smaller image (256kb limit, really?).
>
> *From: *Andy Goryachev <andy.goryachev at oracle.com>
> <mailto:andy.goryachev at oracle.com>
> *Date: *Tuesday, October 10, 2023 at 10:49
> *To: *Michael Strauß <michaelstrau2 at gmail.com>
> <mailto:michaelstrau2 at gmail.com>
> *Cc: *openjfx-dev at openjdk.org <openjfx-dev at openjdk.org>
> <mailto:openjfx-dev at openjdk.org>
> *Subject: *Re: [Request for Comments] Behavior / InputMap
>
> Dear Michael:
>
> Here is a use case for (re-)mapping by the user at runtime:
>
> (key mappings UI in Eclipse).
>
> I can think of several other cases (mentioned in the proposal,
> I think) so I think we can put the concept of immutable or
> global InputMap to rest.
>
> Whether the InputMap contains the reference to its control or
> not is a minor implementation detail, I think.
>
> -andy
>
> *From: *openjfx-dev <openjfx-dev-retn at openjdk.org>
> <mailto:openjfx-dev-retn at openjdk.org> on behalf of Michael
> Strauß <michaelstrau2 at gmail.com> <mailto:michaelstrau2 at gmail.com>
> *Date: *Tuesday, October 10, 2023 at 10:36
> *To: *
> *Cc: *openjfx-dev at openjdk.org <openjfx-dev at openjdk.org>
> <mailto:openjfx-dev at openjdk.org>
> *Subject: *Re: [Request for Comments] Behavior / InputMap
>
> > Yes, one of the features the new design provides is ability to modify
> key mappings by the user at runtime. So yes, not only it
> needs to be mutable, but it also adds some APIs for exactly that.
> >
>
> I struggle to see a use case for this feature. I can imagine that
> there might be some use cases that call for customized input
> mappings,
> but why would this translate to a _mutable_ input map? That's
> quite a
> departure from the way other parts of JavaFX work.
>
> For example, skins are also immutable. If you want to have a
> different
> skin for a control, you don't somehow modify the existing skin
> instance; instead, you'd create a new skin class (or -- somehow --
> extend an existing skin class), and then install that new skin
> on your
> control.
>
> An input map shouldn't bind input events directly to instance
> methods
> of a particular control instance. It should define the mapping of
> events to methods symbolically:
>
> Instead of mapping Event => instance.method(), it should map
> Event =>
> Control::method. The input map could then be stateless and
> immutable,
> and can be set on any control instance. If you want to change the
> mappings, just set a different input map instance. There's no need
> that an input map would retain a reference to any particular
> control,
> since the control reference can be passed into the input map
> just as
> easily.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20231014/8c02a3e5/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image002.jpg
Type: image/jpeg
Size: 104433 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20231014/8c02a3e5/image002-0001.jpg>
More information about the openjfx-dev
mailing list