<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi Andy,<br>
</p>
<p>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.</p>
<p>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.</p>
<p>Observations:</p>
<p>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).</p>
<p>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). <br>
</p>
<p>Analysis:</p>
<p>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).</p>
<p>Solution:</p>
<p>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).</p>
<p>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.<br>
</p>
<p>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
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.org/browse/JDK-8337246">https://bugs.openjdk.org/browse/JDK-8337246</a> without further hacks.<br>
</p>
<p>Takeaways:</p>
<p>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).</p>
<p>Event handler order when Skins get re-added:</p>
<p>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:</p>
<p>- 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).<br>
- 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</p>
<br>
<div class="moz-cite-prefix">On 07/11/2024 01:03, Andy Goryachev
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:BL3PR10MB6185AAAD30E56D358CF52E02E5532@BL3PR10MB6185.namprd10.prod.outlook.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<meta name="Generator"
content="Microsoft Word 15 (filtered medium)">
<style>@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}@font-face
{font-family:Aptos;
panose-1:2 11 0 4 2 2 2 2 2 4;}@font-face
{font-family:"Iosevka Fixed SS16";
panose-1:2 0 5 9 3 0 0 0 0 4;}@font-face
{font-family:"Times New Roman \(Body CS\)";
panose-1:2 11 6 4 2 2 2 2 2 4;}@font-face
{font-family:"Helvetica Neue";
panose-1:2 0 5 3 0 0 0 2 0 4;}p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:10.0pt;
font-family:"Aptos",sans-serif;}a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Iosevka Fixed SS16";
color:windowtext;}p.li1, li.li1, div.li1
{mso-style-name:li1;
margin:0in;
font-size:10.0pt;
font-family:"Helvetica Neue";}span.apple-converted-space
{mso-style-name:apple-converted-space;}.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
mso-ligatures:none;}div.WordSection1
{page:WordSection1;}ol
{margin-bottom:0in;}ul
{margin-bottom:0in;}</style>
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">Dear
Michael:<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">What
happened to this proposal? I would like to restart the
discussion, if possible.<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">More
specifically, I would like to discuss the following topics:<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<ol style="margin-top:0in" type="1" start="1">
<li class="li1" style="mso-list:l0 level1 lfo1"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">the
reason the discussion was started was due to "priority
inversion" problem in Controls/Skins, ex.:
<a href="https://bugs.openjdk.org/browse/JDK-8231245"
moz-do-not-send="true">JDK-8231245</a> 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?<o:p></o:p></span></li>
<li class="li1" style="mso-list:l0 level1 lfo1"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">dispatching
of events that have been consumed (as mentioned in the
earlier discussion)<o:p></o:p></span></li>
<li class="li1" style="mso-list:l0 level1 lfo1"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">problem
of creating unnecessary clones of events via
Event.copyFor(), leading to ex.:
<a href="https://bugs.openjdk.org/browse/JDK-8337246"
moz-do-not-send="true">JDK-8337246</a> SpinnerSkin does
not consume ENTER KeyEvent when editor ActionEvent is
consumed<o:p></o:p></span></li>
<li class="li1" style="mso-list:l0 level1 lfo1"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">why
do we need Event.copyFor() in the first place?<span
class="apple-converted-space">
</span>why does Event contain the target??</span></li>
</ol>
</div>
</blockquote>
<p>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).</p>
<p>2. This is IMHO a bug, it only happens for the same event type at
one level, it should simply be fixed</p>
<p>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.</p>
<p>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).<br>
</p>
<p>--John<br>
</p>
<blockquote type="cite"
cite="mid:BL3PR10MB6185AAAD30E56D358CF52E02E5532@BL3PR10MB6185.namprd10.prod.outlook.com">
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">Too
many topics, yes, we might want to split the discussion into
separate threads.<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">-andy<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<div id="mail-editor-reference-message-container">
<div>
<div>
<div
style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span
style="font-size:12.0pt;color:black">From:
</span></b><span
style="font-size:12.0pt;color:black">openjfx-dev
<a class="moz-txt-link-rfc2396E" href="mailto:openjfx-dev-retn@openjdk.org"><openjfx-dev-retn@openjdk.org></a> on behalf of
Michael Strauß <a class="moz-txt-link-rfc2396E" href="mailto:michaelstrau2@gmail.com"><michaelstrau2@gmail.com></a><br>
<b>Date: </b>Friday, October 27, 2023 at 19:41<br>
<b>To: </b>openjfx-dev
<a class="moz-txt-link-rfc2396E" href="mailto:openjfx-dev@openjdk.org"><openjfx-dev@openjdk.org></a><br>
<b>Subject: </b>Re: Prioritized event handlers<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt">Here
is the proposal:<br>
<a
href="https://gist.github.com/mstr2/4bde9c97dcf608a0501030ade1ae7dc1"
moz-do-not-send="true"
class="moz-txt-link-freetext">https://gist.github.com/mstr2/4bde9c97dcf608a0501030ade1ae7dc1</a><br>
<br>
Comments are welcome.<br>
<br>
<br>
On Fri, Oct 27, 2023 at 8:21</span><span
style="font-size:11.0pt;font-family:"Arial",sans-serif"> </span><span
style="font-size:11.0pt">PM Andy Goryachev<br>
<a class="moz-txt-link-rfc2396E" href="mailto:andy.goryachev@oracle.com"><andy.goryachev@oracle.com></a> wrote:<br>
><br>
> Would it be possible to create a proposal in
the JEP format outlining the proposed public API?<br>
><br>
><br>
><br>
> Thank you<br>
><br>
> -andy<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</body>
</html>