RFR: 8204 : Keyboard traps in flamegraph view for search option [v3]
Brice Dutheil
bdutheil at openjdk.org
Wed Feb 28 10:49:52 UTC 2024
On Tue, 27 Feb 2024 16:56:14 GMT, Virag Purnam <vpurnam at openjdk.org> wrote:
>> Focus Trapped Issue. Keyboard focus is trapped in three specific scenarios mentioned below:
>>
>> 1. Focus is trapped in JMC Agent Preset Manager
>>
>> Select Window -> JMC Agent Preset Manager
>> Click on "Import Fils..."/ "Export File" button
>> Press Esc button when dialog is displayed
>> Press "Tab" button for multi times
>>
>> 2. Focus is trapped in Description of Template Manager
>>
>> Select JVM Browser
>> Start Flight Recording
>> Template Manager
>> Click New
>> Click Edit
>> Focus to Description
>>
>> 3. Focus is trapped in search tool in Flame Graph (Mostly on Windows)
>>
>> Launch JDK Mission Control
>> Open any JFR file
>> Navigate to "Outline" tab
>> In Java Application, choose "Threads"
>> Move focus to search tool in Flame Graph
>>
>> Solution:
>> 1. PresetManagerPage.java : Setting the focus back to tableInspector.
>> 2. TemplateEditPage.java : Added TraverseListener() for description field.
>> 3. FlamegraphSwingView.java : Fix is not straight forward as SWT to Swing and back to SWT traversal is creating issue here. I have just implemented work around for this. I have added KeyListener() to all the swing components. Once it traverse through all the components, I am setting the Focus back to SWT. With this there is no keyboard focus trap.
>>
>> These issues are kind of blocker with respect to accessibility. Could someone please review the changes.
>
> Virag Purnam has updated the pull request incrementally with one additional commit since the last revision:
>
> 8204: Keyboard traps in flamegraph view for search option
LGTM albeit a few things to fix.
Not tested on either Linux or Windows.
application/org.openjdk.jmc.flightrecorder.flamegraph/src/main/java/org/openjdk/jmc/flightrecorder/flamegraph/views/FlamegraphSwingView.java line 523:
> 521: // Adding focus traversal and key listener for main panel
> 522: setFocusTraversalProperties(panel);
> 523: addKeyListenerForBkwdFocustoSWT(panel);
**typo:** Small typo here on `toSWT`
Suggestion:
addKeyListenerForBkwdFocusToSWT(panel);
application/org.openjdk.jmc.flightrecorder.flamegraph/src/main/java/org/openjdk/jmc/flightrecorder/flamegraph/views/FlamegraphSwingView.java line 681:
> 679: if (e.getKeyCode() == KeyEvent.VK_TAB) {
> 680: // Shift + TAB
> 681: if (e.getModifiersEx() > 0) {
**suggestion:** Same suggestion about using the mask instead of a comment.
-------------
PR Review: https://git.openjdk.org/jmc/pull/554#pullrequestreview-1905903822
PR Review Comment: https://git.openjdk.org/jmc/pull/554#discussion_r1505734553
PR Review Comment: https://git.openjdk.org/jmc/pull/554#discussion_r1505745168
More information about the jmc-dev
mailing list