Prioritized event handlers

Fri Nov 8 20:46:45 UTC 2024

This whole situation reminds me very much of Shadow DOM [1], an approach 
for components (controls/skins) with a complex internal structure that 
should not be visible from the outside. The handling of events could 
serve as a template here. [2]

[1] 
https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_shadow_DOM
[2] https://pm.dartus.fr/posts/2021/shadow-dom-and-event-propagation/

On 07.11.24 20:06, Andy Goryachev wrote:
>
> > 4. The target allows you to register the same event handler instance 
> multiple times on multiple different controls without having to create 
> a new lambda each time that knows which control (target) it is for. It 
> has a similar function to the Observable parameter in 
> InvalidationListeners and ChangeListeners.  In effect, having the 
> Target will also make it possible in the future to share a single 
> input map with all controls of the same type (currently there is a ton 
> of memory being wasted here duplicating input maps, especially for 
> controls with large input maps like TextFields).
>
> This is a very strange rationale, in my opinion.  And why talk about 
> Controls when the Event.copyFor() is at the very fundamental level?  
> To me, this severely complicates everything, including breaking 
> isConsumed() logic.  Sending events that are not designed to bubble up 
> seems to run contrary to the whole event dispatching idea.  Are you 
> trying to explain the current implementation?  Swing never needed to 
> store the target in the event, and it works just fine.
>
> This brings me to another problem I am struggling with: why expose the 
> event dispatchers??  This should have never been done, in my opinion.  
> Swing's FwDispatcher is an implementation detail.  But instead, we 
> seem to be dealing with unnecessarily increased complexity, including 
> misbehaving EventDispatcher implementations.
>
> May be it's just me, I don't see the beauty of this thing.
>
> -andy
>
> *From: *openjfx-dev <openjfx-dev-retn at openjdk.org> on behalf of John 
> Hendrikx <john.hendrikx at gmail.com>
> *Date: *Thursday, November 7, 2024 at 05:35
> *To: *openjfx-dev at openjdk.org <openjfx-dev at openjdk.org>
> *Subject: *Re: Prioritized event handlers
>
> Hi Andy,
>
> Simply replacing a Skin can change the order of event handlers.  In 
> other words, if you registered your handler **after** the control was 
> shown, then replace its Skin (which can be done by CSS state changes), 
> then your handler is now called before the Skin/Behavior handlers.  
> Also removing your own handlers, then installing them back (in the 
> same order) will not lead to the same result either.  The culprit is 
> simply that two different parties (FX and the User) are sharing the 
> same system -- this is mostly fine for properties, but Events are 
> stateful, and so the processing order matters.
>
> I've given this quite a lot of thought recently, and I also looked at 
> the Spinner problem, and I think I do have an all encompassing 
> solution for both the ordering problem and the Spinner problem.
>
> Observations:
>
> Controls have some trouble keeping their Skin internals hidden from 
> the outside world.  For the user, the Spinner is a black box; it is 
> the thing that gets the focus and accepts input, even though the 
> actual implementation may be delegating this to an internal component 
> (in the case of the default SpinnerSkin, a FakeFocusTextField -- a 
> detail that you can see "leaking" out when registering a listener on 
> Scene#focusOwner).
>
> Secondly, because Skins are re-using normal controls, which only work 
> when focused, they sometimes have to "fake" the focus (this is a 
> problem I won't be solving in this post).  IMHO however, this should 
> be handled differently altogether, perhaps with some kind of focus 
> delegation mechanism as part of Behaviors -- the user observable focus 
> should IMHO always be the Control, as it is a black box.  The current 
> fake focus implementation tries to keep this hidden (by not firing 
> change listeners) which is incredibly dubious, and also fails (you can 
> still see internals being focused with Scene#focusOwner).
>
> Analysis:
>
> Inside SpinnerSkin, there is a lot of jumping through hoops in the 
> form of filtering events, copying them, and being aware of what the 
> Behavior needs (SpinnerSkin for example knows that the Behavior has 
> need of the UP/DOWN/LEFT/RIGHT keys, it shouldn't...) -- all this 
> jumping through hoops is to make the internal TextField component 
> receive events, but at the same time, not confusing the Behavior with 
> duplicated events (as the Behavior handlers are in between the event 
> root (the window) and the target (the internal textfield) events flow 
> past the Behavior's handlers).
>
> Solution:
>
> The Behavior event handlers are at the Control level -- as far as the 
> user is concerned, this is the target and also final destination 
> (there is nothing deeper).  In effect you could say that the Behavior 
> could register either a Filter or a Handler, and the result would be 
> the same -- filtering down from Scene -> Node -> Spinner and then 
> bubbling up from Spinner -> Node -> Scene, it shouldn't matter if the 
> Behavior registers a filter or a handler as they're at the same 
> position (of course there are some small differences, but I think we 
> actually WANT filters in behaviors, more on this later).
>
> One of the problems is that the event handlers in the behavior trigger 
> **after** the internal TextField -- but we have the perfect mechanism 
> to swap this around; if the Behavior registered a filter (which as I 
> said earlier is sort of at the same position conceptually) then it 
> would trigger **before** the internal TextField.  Spinner is then free 
> to process and consume the arrow keys it needs, and the Skin would not 
> need to be aware of this (ie. if a different Behavior decides to 
> consume the +/- keys, then it would work with the current Skin).  The 
> Skin then just forwards any events that arrive at the Control level 
> still (everything minus potential arrow keys consumed by the Behavior) 
> to the internal text field.
>
> The second part of this solution relates to how to get Events to go 
> down to the internal TextField.  Events are fired at the current focus 
> owner, which is the Control. The TextField only has fake focus.  So we 
> need to take events that arrive at the Control, copy them and send 
> them to the internal TextField.  However, there is a small problem -- 
> the standard event mechanism will make such copies filter down from 
> Window level all the way to the TextField level (passing the Spinner 
> control).  But this isn't want we want at all.  Spinner is a black 
> box; when we want to fire an event into the internals, we don't want 
> this event to be exposed either (aside from the problems that causes, 
> with having to be careful not to copy the event again as it will pass 
> your forwarding handler again).  In effect, we want a different event 
> dispatch hierarchy that is limited to the Control only. So, why not 
> build a dispatch chain that exclusively targets the internals?  This 
> chain in this case only consists of one EventDispatcher, the one from 
> the internal TextField.  The event copy is fired at it and the control 
> works as expected.  It automatically also solves the "ENTER" problem 
> described in https://bugs.openjdk.org/browse/JDK-8337246 without 
> further hacks.
>
> Takeaways:
>
> I now think Behaviors should always be doing their event handling in 
> filters, for the simple reason that they should get to act first 
> before the Skin gets to act on an event; events arriving at the 
> Control level which is the final destination can be handled by either 
> a handler or a filter and there would be no discernible difference.  
> Not only does this solve the problem with the internal text field and 
> the Skin needing to be aware of what keys Behavior will be handling, 
> but it also then makes behaviors always act before user event 
> handlers, creating consistency here (ie. replacing a Skin will not 
> affect event handler order).  This frees up handlers for user 
> purposes.  The ordering problem moves to the filters, so we may still 
> want to do a solution here (see below).
>
> Event handler order when Skins get re-added:
>
> Handlers registered by users can change order with regards to handlers 
> registered by Behaviors when the Skin change or depending on when the 
> user registered their handlers.  This is problematic for events as 
> they are stateful and can be consumed, and so an order change can lead 
> to a functionality change. This is not user friendly, as the user 
> expects that the event handler and filter infrastructure is for their 
> use alone.  I can see a few things we could explore further:
>
> - See if it is possible to add some internal API that allows Behaviors 
> to register filters or handlers first/last -- note that "last" here 
> means **always** last, even when user handlers are added later (first 
> would be an easier option).
> - See if it is possible to change how `buildEventDispatchChain` works 
> for Controls to include another dispatcher level exclusively for 
> Behaviors. Where a standard chain would look like 
> Window->Scene->Nodes->Control, it would become 
> Window->Scene->Nodes->Control->Behavior(private) -- as this is a 
> completely separate level, handlers from users and behaviors cannot 
> become intertwined and so there will always be a predictable ordering
>
> On 07/11/2024 01:03, Andy Goryachev wrote:
>
>     Dear Michael:
>
>     What happened to this proposal?  I would like to restart the
>     discussion, if possible.
>
>     More specifically, I would like to discuss the following topics:
>
>      1. the reason the discussion was started was due to "priority
>         inversion" problem in Controls/Skins, ex.: JDK-8231245
>         <https://bugs.openjdk.org/browse/JDK-8231245> Controls'
>         behavior must not depend on sequence of handler registration. 
>         Do we have this problem elsewhere?  In other words, does it
>         make sense to change the API at the EventDispatcher level when
>         the problem can be easily solved by the InputMap at the
>         Control level?
>      2. dispatching of events that have been consumed (as mentioned in
>         the earlier discussion)
>      3. problem of creating unnecessary clones of events via
>         Event.copyFor(), leading to ex.: JDK-8337246
>         <https://bugs.openjdk.org/browse/JDK-8337246> SpinnerSkin does
>         not consume ENTER KeyEvent when editor ActionEvent is consumed
>      4. why do we need Event.copyFor() in the first place?why does
>         Event contain the target??
>
> 1. You would have the same problem with other events that Behaviors 
> listen to (ie. mouse events), or events they handle without going 
> through the input map (TYPED, RELEASED), or events they handle with a 
> parent event type (all KeyEvents, not a specific one).
>
> 2. This is IMHO a bug, it only happens for the same event type at one 
> level, it should simply be fixed
>
> 3. See above, but in short, I think the problem is not the event 
> clones, but the fact that these clones are traversing the entire 
> Window->Scene->Node hierarchy.  Events with a restricted hierarchy 
> solve this.
>
> 4. The target allows you to register the same event handler instance 
> multiple times on multiple different controls without having to create 
> a new lambda each time that knows which control (target) it is for. It 
> has a similar function to the Observable parameter in 
> InvalidationListeners and ChangeListeners.  In effect, having the 
> Target will also make it possible in the future to share a single 
> input map with all controls of the same type (currently there is a ton 
> of memory being wasted here duplicating input maps, especially for 
> controls with large input maps like TextFields).
>
> --John
>
>     Too many topics, yes, we might want to split the discussion into
>     separate threads.
>
>     -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: *Friday, October 27, 2023 at 19:41
>     *To: *openjfx-dev <openjfx-dev at openjdk.org>
>     <mailto:openjfx-dev at openjdk.org>
>     *Subject: *Re: Prioritized event handlers
>
>     Here is the proposal:
>     https://gist.github.com/mstr2/4bde9c97dcf608a0501030ade1ae7dc1
>
>     Comments are welcome.
>
>
>     On Fri, Oct 27, 2023 at 8:21 PM Andy Goryachev
>     <andy.goryachev at oracle.com> <mailto:andy.goryachev at oracle.com> wrote:
>     >
>     > Would it be possible to create a proposal in the JEP format
>     outlining the proposed public API?
>     >
>     >
>     >
>     > Thank you
>     >
>     > -andy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20241108/6edbcaab/attachment-0001.htm>