Andy, John Hendrikx has given specific instances where this is breaking down already. The JavaDoc suggest that you can handle events on a node by installing event handlers. What it does not say is that a control may have already installed some unknown number of event handlers and there’s no guarantee yours will go first. And the key to controlling the customization of event handling is to get there first.
The same problem occurs when subclassing a control (and I’m assuming we want to make it easier for clients to do that). Any one of the superclasses may have installed an event handler on the Node. If my subclass installs the same handler the two handlers will be executed in some random order which may or may not work. In other words, we have an existing problem with contention in the Node’s dispatcher and it will get worse. There’s nothing here that’s *impossible* to solve using the existing machinery. In both instances there are ways of manipulating the event dispatch chain to do what needs to be done. But it is not easy to write a custom EventDispatcher. It has to be coded correctly to support the bubbling and capturing phases and there is special handling for mouse enter/exit events. So if we want to pursue dispatchers as a solution we need to provide one. (Even if we don’t export an EventHandlerManager we have our own internal reasons to use one. Any time a control drops an event handler on the Node it will interact with every other event handler there. So if new code adds a new handler we should view that as a significant behavioral change that affects external clients. I don’t think we want to be in that position.) Martin > On Oct 16, 2023, at 11:31 AM, Andy Goryachev <andy.goryac...@oracle.com> > wrote: > > 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 > <mailto: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 3rd 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-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 <mailto: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 <mailto: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 <mailto: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.
