Martin:
What would be the use case for creating a public EventHandlerManager?
In general, what is the problem we are trying to address that is
impossible to solve using the existing machinery?
Thank you
-andy
*From: *Martin Fox <mar...@martinfox.com>
*Date: *Saturday, October 14, 2023 at 14:48
*To: *John Hendrikx <john.hendr...@gmail.com>
*Cc: *Andy Goryachev <andy.goryac...@oracle.com>,
openjfx-dev@openjdk.org <openjfx-dev@openjdk.org>
*Subject: *Re: [External] : Re: [Request for Comments] Behavior / InputMap
I’ve been digging around in the code to get some context on the
existing machinery for ordering event handler execution. I haven’t had
time to write up test cases so all of this is based on reading the
spec and the code.
The current problem is that all handlers for a given Node are thrown
into the same bucket and that bucket isn't delivering the execution
order clients expect. Within the existing framework the minimal
solution would be to (a) segregate handlers into separate buckets and
(b) control the order in which the buckets are processed.
The second problem is solved. The order of execution is controlled by
the EventDispatchChain and there are public API’s for manipulating it.
Note that it’s possible for one Node to have multiple dispatchers in
the chain and anyone can take a fully constructed Node and add an
EventDispatcher to it (if you’re subclassing a node you would override
buildEventDispatchChain).
Internally the first problem also has a solution. EventDispatchers
which manage event handlers and filters are called
EventHandlerManagers (the bucket I referred to earlier is the Node’s
event handler manager). Again, a Node can have multiple dispatchers
and any of them could be an EventHandlerManager. For example, the
Control class could have an internal EventHandlerManager with its own
collection of handlers and filters that is entirely separate from the
Node’s. It would be up to the class to determine whether its manager
executed before or after the Node’s.
(In the public API anyone can implement an EventDispatcher but if they
want to support the existing calls for managing event handlers and
filters they are on their own. Personally I think we should provide an
EventHandlerManager for public use.)
Martin
On Oct 14, 2023, at 10:19 AM, John Hendrikx
<john.hendr...@gmail.com> wrote:
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.hendr...@gmail.com>
<mailto:john.hendr...@gmail.com>
*Date:*Thursday, October 12, 2023 at 01:33
*To:*Andy Goryachev<andy.goryac...@oracle.com>
<mailto:andy.goryac...@oracle.com>,openjfx-dev@openjdk.org<openjfx-dev@openjdk.org>
<mailto:openjfx-dev@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
tohttps://bugs.openjdk.org/browse/JDK-8091189and 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-r...@openjdk.org>
<mailto:openjfx-dev-r...@openjdk.org>on behalf of John
Hendrikx<john.hendr...@gmail.com>
<mailto:john.hendr...@gmail.com>
*Date:*Wednesday, October 11, 2023 at 01:04
*To:*openjfx-dev@openjdk.org<openjfx-dev@openjdk.org>
<mailto:openjfx-dev@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.goryac...@oracle.com>
<mailto:andy.goryac...@oracle.com>
*Date:*Tuesday, October 10, 2023 at 10:49
*To:*Michael Strauß<michaelstr...@gmail.com>
<mailto:michaelstr...@gmail.com>
*Cc:*openjfx-dev@openjdk.org<openjfx-dev@openjdk.org>
<mailto:openjfx-dev@openjdk.org>
*Subject:*Re: [Request for Comments] Behavior / InputMap
Dear Michael:
Here is a use case for (re-)mapping by the user at
runtime:
<image002.jpg>
(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-r...@openjdk.org>
<mailto:openjfx-dev-r...@openjdk.org>on behalf of
Michael Strauß<michaelstr...@gmail.com>
<mailto:michaelstr...@gmail.com>
*Date:*Tuesday, October 10, 2023 at 10:36
*To:*
*Cc:*openjfx-dev@openjdk.org<openjfx-dev@openjdk.org>
<mailto:openjfx-dev@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.
