4512626: Non-editable JTextArea provides no visual indication of keyboard focus #11408
Conversation
…oard focus Set the text caret to be visible but not blinking on the non-editable text area.
azuev-java
changed the title
|
👋 Welcome back kizune! A progress list of the required criteria for merging this PR into |
|
@azuev-java The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
|
Hmm. This is a pretty broad change. |
| if (savedBlinkRate != 0) { | ||
| setBlinkRate(savedBlinkRate); | ||
| savedBlinkRate = 0; | ||
| } |
There was a problem hiding this comment.
My general questions need to be addressed first, but after we get past that you need to revisit this code.
It seems to overlook that an application could change the blink rate itself, and restores the saved rate over what
the application has set.
Also, SFAICT setBlinkRate() doesn't seem to be preventing negative values ...
There was a problem hiding this comment.
My general questions need to be addressed first, but after we get past that you need to revisit this code. It seems to overlook that an application could change the blink rate itself, and restores the saved rate over what the application has set.
Ok, the only way it can be a problem is when user will try to set the blink rate of the uneditable component's caret. I think i will have to special-case that.
Also, SFAICT setBlinkRate() doesn't seem to be preventing negative values ...
I wonder what will happen if i set a negative rate. Will it blink into the past before component is even created? Can it cause a time paradox? Hmm.
They are also affected in the same way.
The bug was reported during the VPAT certification of the JCK suite. They use a lot of JTextArea for the testing instructions and looks like that is what caught attention of the A11Y office. Would they notice the JTextField issue they would've report that too.
Same. They are affected by the issue too.
You mean native Windows controls? I have to check it out - and not only on Windows. However that would not affect our A11Y certification.
Have to check other components but JLabel can not receive focus while non-editable JTextArea and JTextField can. Moreover - the caret is active, you can move it and when editable status changes the caret will be in a new position. If you move it with shift pressed - you can select text and with corresponding shortcuts you can copy that selected text to the clipboard. |
|
I wonder how other native applications handle this. Can the non-blinking cursor be considered a hang of the application? |
Test HidingSelectionTest.java is rewritten and corrected.
Different application handle it differently but most os them do not claim to pass the VPAT certification. I do not think that non-blinking cursor can be treated as hanged application. |
|
I added the setBlinkRate handling when component is unedited so the new rate is preserved. Also setting blink rate to a negative value causes InvalidArgumentException because it is checked further down the line in javax.swing.Timer.checkDelay() method so it is a non-issue. Finally the HidingSelectionTest had to be re-written because its logic is incorrect and it passes on MacOS and Windows only because of the flaw in the test that allows false positive. |
|
At least on Windows, non-editable text controls have a blinking caret. Just open Properties of any file in Windows Explorer. All the information is displayed in non-editable text controls except for the file name which is in an editable one. The non-editable text controls in Properties dialog have no borders, so no other indication but the blinking caret indicates the control is focused. It is possible to add the border to a text control. I don't think a non-blinking caret is a good idea. |
Well, Windows is an important platform but it is not the only one we care about. On Ubuntu 22 the editable text field has the blinking cursor and non-editable has a non-blinking - to indicate that text can not be entered or changed but can be selected and copied and that's just one of examples of the difference in behaviour on different platforms. Based on that i would say that as a common ground i propose the non-blinking cursor on non-editable text components and if some platform needs an adjustments - we can always file a bug and override that in the platform-specific LaF like AquaCaret does. |
The test is problemlisted for windows so not sure if the observation "passes on Windows" is correct..if it is, please deproblemlist it.. |
How that different applications handle that? |
Chrome form editor hides cursor, Adobe Acrobat shows blinking cursor, Microsoft Office 2011 shows some custom cursor that appears to be grayed off or faded. Differently. |
How do native apps on macOS handle the situation? Especially those developed by Apple? If two platforms show blinking caret, then the caret should remain blinking by default. |
It is quite hard to find a relevant case but so far it seems that it is all over the place on Mac OS - in majority of the apps where it allows selection in non-editable text components with keyboard it does not show the caret at all. I found only one place that shows caret in non-editable text field - in the project settings of XCode app - and the caret is blinking but i can not say if it is a normal behaviour or if it was somehow customized. My point is that if editable text component can not be visually distinguished from non-editable one it is also not helpful. Like on that XCode page - there are a set of text fields some of them are editable and some of them are not and the only way to distinguish between them is to attempt to type or delete text in them. Honestly so far Linux/Gnome approach just looks more coherent and logical. |
When it failed on Linux on pre-submit task i ran it locally and deproblemlisted it on Windows. I haven't pushed the problemlist change but i found that on Windows it is unstable because of the way it was taking screenshots and it was passing because it compared the two screenshots that are not supposed to be the same and it passes because they are not but that is the false positive because the difference is not in the relevant part of the screenshot but in the border overlapped by the popup. Except this false positive is not a false positive because on Windows these components should be the same for the logic tested is only valid on Linux - selection of the text in the focused window does not clear selection in non-focused one on Windows or Mac. So in the similar fashion test "accidentally" passes on Mac but for different reason - the screenshots were different because on Mac the selection color in non-focused window is different so screenshot does not match while functionally selection is not cleared so it is another "false but correctly positive". |
Agreed. Does the background colour* of non-editable text component change in Swing? If not, then non-blinking caret is the way to go so that it contrasts with the editable one. * On Windows, such controls are usually displayed with darker background, like a static control which corresponds to |
It does not. |
Does any of them use non blinking cursor? it seems related to the application and not the library they use. The library provides a way to set different cursors and the application may use that. Maybe we can update the SwingSet2 which is used for testing to show that it is possible to do that in Swing? We updated SwingSet2 in the past for a similar reason: https://bugs.openjdk.org/browse/JDK-8278604 and https://bugs.openjdk.org/browse/JDK-8278526 |
|
BTW the description of this bug states:
I think that using the VoiceOver or JAWS will highlight the currently focused component since we should send that information to them, no? |
|
And I do not think this statement in the bug description is true:
|
The library does not provide an ability to set caret to visible permanently in case of text component being non-editable. Every time focus will change the caret will change too. One would have to implement a custom caret that overrides this logic. |
That paragraph consists of two statements - first is that focus should be visible and one can tell which component has focus without the use of VO or JAWS. The second part is that it should be also exposed trough the assistive technology API is not related to the problem at hand. |
It might have been true when the bug was submitted. I mean it is 21 years old, soon it might have its own small bugs. |
If that is not possible I suggest providing that functionality so applications will be able to use that. Probably even provide some predefined cursors, like "faded"/"blurred"/nonblinked/etc. That could be configured per L&F as well. |
That would be a nice feature to have but right now we have an issue where default behaviour of the component defies the VPAT guidelines. We can argue about the type of cursor to use but lack of caret makes focus invisible and keyboard selection - while possible - very clunky. So i stay by my decision to avoid hiding the caret and make it non-blinking so we can easily distinguish between editable and non-editable text components. If and when in the future we add the better caret support for text states we might make the default non-editable caret faded or dithered or something along the lines but for now i propose this. |
I am not sure that changing the default behavior of all text components(including embedded to other components) to use a non-blinking cursor is a good idea, you can convince me if you find someone who uses the same approach, any applications? Probably at least for the native look&feels we can use the same behavior as the native default components win32/cocoa/gtk. |
Gnome on Ubuntu 22, default applications. Editable text components have blinking carets, non-editable - non-blinking ones. |
|
I agree with @azuev-java. A caret — blinking or not — is better than no caret at all. The caret not only visually shows the focus owner (a non-editable text component) but also facilitates text selection (with keyboard). |
I believe this has been discussed here:
The above observations are based on the File Properties dialog in each OS. So, the app developer should have control over the behaviour. Then Look-and-Feel implementation could have its own defaults which match the platforms if applicable. At the same time, I don't this limitation as a showstopper. As I said above, a visible caret facilitates selecting text using keyboard; from this point of view, this fix is a step forward. |
|
@azuev-java This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 260 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
|
I created an enhancement request JDK-8298602 to start a discussion about the state-aware caret implementation so we can come to a more flexible solution in the future. With that in mind can we come to some conclusion on this exact accessibility issue? I still thing it is beneficial to have a way to distinguish between focused/non-focused and editable/non-editable text components and the caret seems to be potentially less destructive change. I do not see any major UI element that has the text component embedded that is affected in the negative way by this change. |
|
Since jdk20 branched out and we are trying to fix it for jdk20 the new PR is here: openjdk/jdk20#21 |
Set the text caret to be visible but not blinking on the non-editable text area.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/11408/head:pull/11408$ git checkout pull/11408Update a local copy of the PR:
$ git checkout pull/11408$ git pull https://git.openjdk.org/jdk pull/11408/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 11408View PR using the GUI difftool:
$ git pr show -t 11408Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/11408.diff