On Tue, 31 Mar 2026 19:16:55 GMT, Martin Fox <[email protected]> wrote:
>> Michael Strauß has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Avoid variable reassignment > > @maran23 I’ve never paid much attention to the Node focused property since it > doesn’t play any role in the delivery of key events. It would be really > helpful if you could give a brief summary of what it does and, more > importantly, how it’s used. > > Based on your notes it looks like text input controls consult this property > to determine when to show their carets. In the context of this PR this means > that the “focused” property needs to be set on the focus owner’s delegate > (the TextField) rather than the focus owner (the Spinner or ComboBox) as the > spec currently reads. > > This is just another part of the system that needs to be shifted from the > focus owner to the delegate. We also need to use the delegate as the target > for Key and InputMethod events and ensure it’s consulted for input method > requests. > > (I do wonder if it would make more sense for Spinner and ComboBox to set the > focus owner to the TextField instead. This seems to be how the system was > designed to function. The focused property would work as expected, the > pseudo-classes would work as expected, events would automatically be targeted > at the right control, input method requests would work. Even the > accessibility API seems to be designed for this; the FOCUS_ITEM accessibility > property makes it easy to retrieve the Spinner or ComboBox from the > TextField.) > > It also looks like container controls like TableView are setting the > “focused” property on their cells. Why? To add the “focus” pseudo-class to > the cell so it displays a focus ring? Whatever TableView is doing with > “focused” what’s your proposed replacement? @beldenfox, @mstr2 and my idea is to deprecate `setFocused(..)`. Because it does not work well with `focusWithin` (as can be seen in the table right now). It is hacky, the focus is not cleared automatically (You basically need to do everything manually), which makes it confusing. Especially as we have `requestFocus`. It probably exists to workaround the problems I mentioned above. So those need a proper fix. In an old ticket somewhere, I did read something about 'sub focus'. So back then, something like that was planned it looks like. > This is just another part of the system that needs to be shifted from the > focus owner to the delegate. We also need to use the delegate as the target > for Key and InputMethod events and ensure it’s consulted for input method > requests. Yes. I don't know how (a delegate system is one idea), but this needs a proper solution for that reasons. > (I do wonder if it would make more sense for Spinner and ComboBox to set the > focus owner to the TextField instead. This seems to be how the system was > designed to function. The focused property would work as expected, the > pseudo-classes would work as expected, events would automatically be targeted > at the right control, input method requests would work. Even the > accessibility API seems to be designed for this; the FOCUS_ITEM accessibility > property makes it easy to retrieve the Spinner or ComboBox from the > TextField.) The problem with that is that Controls like the `ComboBox` have a listener bound to the `focusedProperty` (and probably a lot of user code out there as well). So as soon as the `TextField` with be the focus owner, the Control thinks the user clicked somewhere else and commit their value. > It also looks like container controls like TableView are setting the > “focused” property on their cells. Why? To add the “focus” pseudo-class to > the cell so it displays a focus ring? Whatever TableView is doing with > “focused” what’s your proposed replacement? My idea is to set the pseudoclass manually (for backward compatibility with CSS) and just decide whether to commit the value or not. No need to use the `focusedProperty` at all. With that fixed, we can utilize the `focusWithin` CSS style to always show the focus ring even if something inside the table is focus owner, like a `TextField` or even a Cell, if a developer makes it focus traversable (default is false right now). This enables us to remove all workarounds that try to keep/shift the focus just for the sake of the focus ring (like the `ScrollBar` ot table headers, that always try to shift the focus away just for the focus ring). ------------- PR Comment: https://git.openjdk.org/jfx/pull/1632#issuecomment-4178744758
