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-r...@openjdk.org> on behalf of John
Hendrikx <john.hendr...@gmail.com>
*Date: *Thursday, November 7, 2024 at 05:35
*To: *openjfx-dev@openjdk.org <openjfx-dev@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-r...@openjdk.org>
<mailto:openjfx-dev-r...@openjdk.org> on behalf of Michael Strauß
<michaelstr...@gmail.com> <mailto:michaelstr...@gmail.com>
*Date: *Friday, October 27, 2023 at 19:41
*To: *openjfx-dev <openjfx-dev@openjdk.org>
<mailto:openjfx-dev@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.goryac...@oracle.com> <mailto:andy.goryac...@oracle.com> wrote:
>
> Would it be possible to create a proposal in the JEP format
outlining the proposed public API?
>
>
>
> Thank you
>
> -andy
