Prioritized event handlers
John Hendrikx
john.hendrikx at gmail.com
Tue Nov 12 19:27:28 UTC 2024
On 12/11/2024 19:46, Nir Lisker wrote:
> I'd like to understand the focus and event handling problem better. If
> I have a focused TextField, all key events go to it. If I have a
> Spinner, which is a TextField with 2 Buttons, it is focused as a
> single unit and consumes key events whether they are aimed at the text
> field or the buttons (I assume the buttons handle arrow up/down
> keys?). If I have a ClolorPicker, it is not focused as a single unit -
> it has sliders, buttons, text fields and other things, which can be
> focused individually.
For Spinner, all key events go to the main Spinner control, including
the arrow keys. This is because KeyEvents are always dispatched to
Scene#focusOwner which the user should never see to be something other
than the Spinner (ie. it shouldn't ever be Spinner Up Button). The
Behavior installed should act first on any such keys, including the
arrow keys. The reason there are no arrow key handlers on the buttons
is because the buttons are Skin specific, and so handling them there
would break the Spinner's intended functionality if those buttons don't
exist or are implemented differently. The arrow keys will work
regardless if there are buttons or not, and so it is not a good idea to
have the Skin handle them (aside from the fact that the Behavior should
be deciding such matters, not the Skin).
However, if the TextField has focus (it really doesn't have it, hence
the FakeFocusTextField) then to make the TextField actually work, the
KeyEvents that are still going to Spinner must be forwarded to the inner
TextField. This is a Skin reponsibility as only the skin knows where
those events could possibly go (if there is no TextField, then nothing
needs forwarding). The current implementation however does this in a
really round-about way, and will resend the KeyEvent from the top of the
Scene graph all the way back to the inner TextField as target -- this
means any handlers in between see these events twice. What should have
been done is that the event is only forwarded locally (I tested this in
a draft PR, and that works splendidly). Michael's proposal essentially
makes this standard functionality, so you don't need to do any manual
forwarding anymore.
ColorPicker is probably also faking the focus on its individual
components (I didn't check), but if it is supposed to be a reusable
integrated Control, then from the focus owner perspective, only
ColorPicker ever has the focus (of course, if it opens a PopUp, that's a
new Scene, which can have its own focus owner).
>
> What I'm trying to find out is what is "the primitive" in the
> focus/event handling plan. A TextField and a Spinner are treated as
> primitives, but a ColorPicker and a DatePicker are not. Where does the
> line pass? If I'm a controls author, can I create a Spinner that
> allows focusing/event-handling the text field and the buttons
> separately, like ColorPicker allows? In this case, Spinner is not a
> "primitive" control.
IMHO ColorPicker and DatePicker are also primitive controls. I want to
be able to tab passed one without going into its inner buttons/fields,
same as combo box. If they open a Popup (after pressing ENTER or SPACE)
then the whole control is still focused, but there is a 2nd focus inside
the new popup created (we wouldn't want the main control to lose its
focus border as that's really confusing). For Spinner for example, when
its text field has the focus, the focus border is around the entire
control (including the arrow buttons), not just on the TextField area.
--John
>
> On Tue, Nov 12, 2024 at 7:56 PM Kevin Rushforth
> <kevin.rushforth at oracle.com> wrote:
>
> I think this is a good discussion to continue. I have a a couple
> quick comments:
>
>> The first question I would like to resolve is to determine
>> whether the problem exists globally, or only at the controls
>> level. If even once scenario exists that does not involve
>> controls, we must find a solution at the event dispatch level.
>> If not - the solution can be at the controls level, and I have
>> proposed a good solution, but it's premature to talk about it
>> right now.
>
> Unless it can be clearly shown that this is a controls-only
> problem, and never will be something that other users of events
> need to worry about, I favor a solution in the event handling
> mechanism itself rather than something controls-specific. So I
> agree with Michael on this point.
>
>> 4, 5. there seems to be general misunderstanding why I see
>> copyFor() as a big problem. (Performance is **not** the issue here).
>
> Very likely. I certainly don't see it as a big problem, which
> suggests I might be missing something. I do find it unlikely that
> we are going to change something as fundamental as having a target
> in the event (which is the main reason for using "copyFor").
>
> -- Kevin
>
>
> On 11/12/2024 8:27 AM, Andy Goryachev wrote:
>>
>> Thank you Michael for answering my questions!
>>
>> I get from your answers that:
>>
>> 1. the priorities are still needed, in one form or the other.
>> Adding a different type of the EH (ifUnconsumed) seems to me like
>> a different priority.
>>
>> 2. the problem seems to exist only at the controls level -
>> nothing was mentioned to cause issues related to priority outside
>> of controls. This seems right, because only in controls we have
>> two (or more) actors engaged in event handling - the application
>> and the skin.
>>
>> 3. dispatching consumed events looks like a bug to all respondents
>>
>> 4, 5. there seems to be general misunderstanding why I see
>> copyFor() as a big problem. (Performance is ***not*** the issue
>> here).
>>
>> Please correct me if I summarized it incorrectly.
>>
>> Another interesting observation is that proposals seem to have
>> been replaced by widely different alternatives - ifUnconsumed and
>> event filters. This might indicate that there is no consensus as
>> of yet, and the discussion must therefore be continued.
>>
>> The first question I would like to resolve is to determine
>> whether the problem exists globally, or only at the controls
>> level. If even once scenario exists that does not involve
>> controls, we must find a solution at the event dispatch level.
>> If not - the solution can be at the controls level, and I have
>> proposed a good solution, but it's premature to talk about it
>> right now.
>>
>> So I would like to ask for clarifications on these three questions:
>>
>> 1. For ifUnconsumed idea: how will it work when both the
>> application and the skin register ifUnconsumed EH? Or is it only
>> available to one side, but not the other?
>>
>> 2. For event filter in behaviors idea: how does it work when both
>> behavior and the application register an event filter? and then
>> the skin is changed? wouldn't we have the same issue?
>>
>> 3. Are there any examples outside of controls where priority
>> inversion happens, or where we need explicit EH priorities for
>> other reasons?
>>
>> Thank you
>>
>> -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, November 8, 2024 at 17:52
>> *To: *
>> *Cc: *openjfx-dev <openjfx-dev at openjdk.org>
>> <mailto:openjfx-dev at openjdk.org>
>> *Subject: *Re: Prioritized event handlers
>>
>> Hi Andy,
>>
>> 1. What happened to this proposal?
>>
>> I've come to the conclusion that we need something like that, but
>> probably in a different form. My current thinking is that we don't
>> need prioritized handlers, but merely a way for interested listeners
>> to say "I'll take this event, but only if no one else wants it".
>> A possible API could be something like the following:
>>
>> target.addEventHandler(KeyEvent.PRESSED, event -> {
>> event.ifUnconsumed(evt -> {
>> // This will be called after the event has bubbled up
>> // without being consumed.
>> });
>> });
>>
>> This will allow skins to act on events only if user code didn't
>> consume them.
>>
>> 2. 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?
>>
>> Yes, because javafx.controls is not a core part of JavaFX, and it
>> should never be. People should be free to create their own controls
>> implementation, or alternative skinning systems. We need to give them
>> the tools to do so, and not continue the anti-pattern of shifting
>> core
>> functionality into javafx.controls and special-casing this module
>> even
>> more than it is already special-cased.
>>
>> 3. dispatching of events that have been consumed (as mentioned in the
>> earlier discussion)
>>
>> Probably not necessary. Once an event is consumed, it's gone; we
>> don't
>> need to dispatch it further.
>>
>> 4. Problem of creating unnecessary clones of events via
>> Event.copyFor()
>>
>> Unless there is a clear performance problem, I consider any
>> fundamental change here as a solution in search of a problem.
>> Events are usually not so plentiful that we're talking about serious
>> CPU cycles here. The highest-frequency events are probably mouse
>> events, and they happen at most hundreds of times per second.
>>
>> 5. If we removed the target, then a listener couldn't discern whether
>> the event was targeted at the receiving node, or at a descendant of
>> the node.
>>
>>
>>
>> On Thu, Nov 7, 2024 at 1:03 AM Andy Goryachev
>> <andy.goryachev at oracle.com> <mailto:andy.goryachev at oracle.com> 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:
>> >
>> > the reason the discussion was started was due to "priority
>> inversion" problem in Controls/Skins, ex.: 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?
>> > dispatching of events that have been consumed (as mentioned in
>> the earlier discussion)
>> > problem of creating unnecessary clones of events via
>> Event.copyFor(), leading to ex.: JDK-8337246 SpinnerSkin does not
>> consume ENTER KeyEvent when editor ActionEvent is consumed
>> > why do we need Event.copyFor() in the first place? why does
>> Event contain the target??
>> >
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20241112/f817fb37/attachment-0001.htm>
More information about the openjfx-dev
mailing list