Hi,
while migrating a larger application to a newer JavaFX version, I noticed
that overriding requestFocus() no longer works reliably since focusVisible
was introduced.
The new feature is great and is an important improvement for JavaFX - only
how it interacts with overridden requestFocus() methods is problematic.
Below is a description of the problem and possible solutions. I'd
appreciate feedback on which one makes sense.
Summary:
Since we have focusVisible with JDK-8274798 / JFX19 the method
requestFocus()
can no longer be used to override what happens - when a node is supposed
to get focus.
This also breaks Apps which migrate from an older JFX Version to a newer
one.
The Problem:
Previously, overriding requestFocus() provided a way to decide what to do
when a node
receives focus. For example, a node could pass the focus on to another one.
But because requestFocus() is no longer called on keyboard input (i.e.
when focusVisible
will be true), the logic of various applications is broken.
This is caused by the new version of
TopMostTraversalEngine.focusAndNotify().
Before, it called requestFocus() unconditionally.
private void focusAndNotify(Node newNode, TraversalMethod method) {
if (method == TraversalMethod.KEY) {
NodeHelper.requestFocusVisible(newNode);
} else {
newNode.requestFocus();
}
// ...
}
Current Workaround in an App:
// PseudoCode
scene.focusOwnerProperty.subscribe(v -> {
if (v != null && v.isFocusVisible) {
// Simulate the no-longer-guaranteed call
v.requestFocus();
}
});
This workaround basically re-calls requestFocus(), which mostly restores
the original
behavior. Ugly, but it works.
Possible Solutions:
# Remember the start state temporarily
- The mode would have to be remembered somewhere during traversal (Scene
member / static / ThreadLocal).
All of these are a bit ugly, but they nevertheless restore the previous
behavior
more precisely than the other solutions.
# Provide a more detailed API
- One could add new methods to the API which are always called before the
focus changes:
// TopLevel somewhere
record FocusMode(boolean focusVisible) {}
// on Node - basically remains for compatibility
public void requestFocus() {
requestFocus(FocusMode.current()); // the mode remembered during
traversal
}
public void requestFocus(FocusMode mode) {
// the recommended place to override.
}
The no-arg method would stay overridable and be the one JavaFX calls
internally, so old overrides keep working.
Overriding it would be discouraged going forward:
the new method carries all the information, so migrating applications
only need to change which version they override.
# Deprecate overriding requestFocus()
- Only declare that requestFocus() should no longer be overridden, because
overriding it
now leads to inconsistent behavior. No migration path is provided.
Nothing to do, except that this should be documented and requestFocus()
should be made
final at some point.
My recommendation would be the more detailed API, since it is the cleanest
long-term solution:
Overriding basically provides the original hook, allowing a clean
migration by changing just one method.
requestFocus() without args basically becomes deprecated and only lives
for compatibility reasons.
Because requestFocus() is already broken - this should not cause
additional problems.
This can be combined with some mechanism to remember the focusMode -
so at least some compatibility with old code is restored which we are
missing today.
It would be great to get some feedback on it.
I would like to provide a concrete implementation suggestion, as a Draft
PR to move the discussion further.
I have filed JDK-8389343 for the regression itself.
Greetings Florian Kirmaier