Question about coalescing COMPONENT_RESIZED events
I’m looking for general feedback/advice.
Is there any reason not to put the following method in a Component?
@Override
protected AWTEvent coalesceEvents(AWTEvent existingEvent, AWTEvent
newEvent) {
if (newEvent.getID() == ComponentEvent.COMPONENT_RESIZED)
return newEvent;
return super.coalesceEvents(existingEvent, newEvent);
}
(That is: is this unsafe/unwise for some reason I’m not considering?)
As I understand it: when you resize a window every call to
component#setBounds(..) generates a new ComponentEvent and posts it to
the event queue. So suppose you resized a window so its height was 10,
then 11, then 12, then 13, then 14: that would create 5 ComponentEvents
and post them to the event queue.
So suppose you also have this listener:
myComponent.addComponentListener(new ComponentAdapter() {
public void componentResized(ComponentEvent e) {
System.out.println(e.getComponent().getHeight());
}
});
Your output in this scenario may be 10, 11, 12, 13, 14 if you have a
very responsive EDT. But it could also be 14, 14, 14, 14, 14. The only
guarantee is that the last printed height of the 5 ComponentEvents will
be 14.
So if the ComponentEvents come in all at once: there’s no harm in
coalescing them, right? (So now you may get anywhere from 1 to 5
ComponentEvents, but each call to e.getComponent().getSize() will
produce a different result.)
Regards,
- Jeremy
Re: RFR: 8331142: Add test for number of loader threads in BasicDirectoryModel
On Thu, 25 Apr 2024 16:37:39 GMT, Alexey Ivanov wrote: > This PR provides a regression test for > [JDK-8325179](https://bugs.openjdk.org/browse/JDK-8325179): _Race in > BasicDirectoryModel.validateFileCache_ reviewed in #18111. > > The test is inspired and based on `ConcurrentModification` that I wrote for > [JDK-8327137](https://bugs.openjdk.org/browse/JDK-8327137) / > [JDK-8323670](https://bugs.openjdk.org/browse/JDK-8323670). > > **How the test works** > > The test creates a temporary directory in the current directory and creates a > number of files in it. (The number of files is controlled by > `NUMBER_OF_THREADS` constant). Then the test creates `JFileChooser` in the > temporary directory. > > The test starts several scanner threads, the number is controlled by > `NUMBER_OF_THREADS`, which repeatedly call > `fileChooser.rescanCurrentDirectory()`. This results in calling > `BasicDirectoryModel.validateFileCache` which starts a background thread — > "Basic L&F File Loading Thread" — to enumerate the files. > > The test runner thread and scanner threads are synchronised with > `CyclicBarrier`, this ensures all the scanner threads start at the same time. > After a short delay, the runner thread takes a snapshot of live threads. > After a longer delay, the operation is repeated. See the `getThreadSnapshot` > method. (The number of snapshots is controlled by `SNAPSHOTS` constant.) > > The number of File Loading Threads in each snapshot is counted. There should > be no more than two threads. It is possible that thread two such threads > after JDK-8325179 is fixed: the existing thread is interrupted but hasn't > exited yet, and a new thread is already created. > > The test fails consistently without the fix for JDK-8325179. On Windows, the > output looks like this: > > > Number of snapshots: 20 > Number of snapshots where number of loader threads: > = 1: 0 > = 2: 0 > > 2: 20 > java.lang.RuntimeException: Detected 20 snapshots with several loading threads > at LoaderThreadCount.runTest(LoaderThreadCount.java:132) > at LoaderThreadCount.wrapper(LoaderThreadCount.java:72) > at java.base/java.lang.Thread.run(Thread.java:1583) > > > On Linux and macOS, there's more variation, for example I got the following > output on one of Linux systems: > > > Number of snapshots: 15 > Number of snapshots where number of loader threads: > = 1: 7 > = 2: 2 > > 2: 6 > > > The test passes on the builds where JDK-8325179 is present. Marked as reviewed by serb (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18957#pullrequestreview-2024039392
Re: RFR: 8303689: javac -Xlint could/should report on "dangling" doc comments [v9]
On Thu, 25 Apr 2024 23:24:07 GMT, Jonathan Gibbons wrote:
>> Please review the updates to support a proposed new
>> `-Xlint:dangling-doc-comments` option.
>>
>> The work can be thought of as in 3 parts:
>>
>> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
>> is possible to specify the appropriately configured `Lint` object when it is
>> time to consider whether to generate the diagnostic or not.
>>
>> 2. Updates to the `javac` front end to record "dangling docs comments" found
>> near the beginning of a declaration, and to report them using an instance of
>> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
>> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
>> procedure for handling dangling doc comments is described in this comment in
>> `JavacParser`.
>>
>> * Dangling documentation comments are handled as follows.
>> * 1. {@code Scanner} adds all doc comments to a queue of
>> * recent doc comments. The queue is flushed whenever
>> * it is known that the recent doc comments should be
>> * ignored and should not cause any warnings.
>> * 2. The primary documentation comment is the one obtained
>> * from the first token of any declaration.
>> * (using {@code token.getDocComment()}.
>> * 3. At the end of the "signature" of the declaration
>> * (that is, before any initialization or body for the
>> * declaration) any other "recent" comments are saved
>> * in a map using the primary comment as a key,
>> * using this method, {@code saveDanglingComments}.
>> * 4. When the tree node for the declaration is finally
>> * available, and the primary comment, if any,
>> * is "attached", (in {@link #attach}) any related
>> * dangling comments are also attached to the tree node
>> * by registering them using the {@link #deferredLintHandler}.
>> * 5. (Later) Warnings may be genereated for the dangling
>> * comments, subject to the {@code -Xlint} and
>> * {@code @SuppressWarnings}.
>>
>>
>> 3. Updates to the make files to disable the warnings in modules for which
>> the
>> warning is generated. This is often because of the confusing use of `/**` to
>> create box or other standout comments.
>
> Jonathan Gibbons has updated the pull request incrementally with one
> additional commit since the last revision:
>
> revert need to disable warning
looks good
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java line
583:
> 581: * dangling comments are also attached to the tree node
> 582: * by registering them using the {@link #deferredLintHandler}.
> 583: * 5. (Later) Warnings may be genereated for the dangling
typo: generated
-
Marked as reviewed by vromero (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18527#pullrequestreview-2023792395
PR Review Comment: https://git.openjdk.org/jdk/pull/18527#discussion_r1580263826
Re: RFR: 8303689: javac -Xlint could/should report on "dangling" doc comments [v9]
> Please review the updates to support a proposed new
> `-Xlint:dangling-doc-comments` option.
>
> The work can be thought of as in 3 parts:
>
> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
> is possible to specify the appropriately configured `Lint` object when it is
> time to consider whether to generate the diagnostic or not.
>
> 2. Updates to the `javac` front end to record "dangling docs comments" found
> near the beginning of a declaration, and to report them using an instance of
> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
> procedure for handling dangling doc comments is described in this comment in
> `JavacParser`.
>
> * Dangling documentation comments are handled as follows.
> * 1. {@code Scanner} adds all doc comments to a queue of
> * recent doc comments. The queue is flushed whenever
> * it is known that the recent doc comments should be
> * ignored and should not cause any warnings.
> * 2. The primary documentation comment is the one obtained
> * from the first token of any declaration.
> * (using {@code token.getDocComment()}.
> * 3. At the end of the "signature" of the declaration
> * (that is, before any initialization or body for the
> * declaration) any other "recent" comments are saved
> * in a map using the primary comment as a key,
> * using this method, {@code saveDanglingComments}.
> * 4. When the tree node for the declaration is finally
> * available, and the primary comment, if any,
> * is "attached", (in {@link #attach}) any related
> * dangling comments are also attached to the tree node
> * by registering them using the {@link #deferredLintHandler}.
> * 5. (Later) Warnings may be genereated for the dangling
> * comments, subject to the {@code -Xlint} and
> * {@code @SuppressWarnings}.
>
>
> 3. Updates to the make files to disable the warnings in modules for which
> the
> warning is generated. This is often because of the confusing use of `/**` to
> create box or other standout comments.
Jonathan Gibbons has updated the pull request incrementally with one additional
commit since the last revision:
revert need to disable warning
-
Changes:
- all: https://git.openjdk.org/jdk/pull/18527/files
- new: https://git.openjdk.org/jdk/pull/18527/files/16a265a2..39689a52
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=08
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=07-08
Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/18527.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18527/head:pull/18527
PR: https://git.openjdk.org/jdk/pull/18527
Re: RFR: JDK-8329004 : Update Libpng to 1.6.43 [v3]
> Libpng updated from 1.6.40 to 1.6.43 Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision: Updating.txt - Changes: - all: https://git.openjdk.org/jdk/pull/18964/files - new: https://git.openjdk.org/jdk/pull/18964/files/f9021883..ad038006 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18964&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18964&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18964.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18964/head:pull/18964 PR: https://git.openjdk.org/jdk/pull/18964
Re: RFR: JDK-8329004 : Update Libpng to 1.6.43 [v2]
On Thu, 25 Apr 2024 22:46:23 GMT, Phil Race wrote: >> Harshitha Onkar has updated the pull request incrementally with one >> additional commit since the last revision: >> >> added missing 4 lines of header > > src/java.desktop/share/native/libsplashscreen/libpng/png.h line 30: > >> 28: * License version 2 only, as published by the Free Software Foundation. >> 29: * However, the following notice accompanied the original version of this >> 30: * file and, per its terms, should not be removed: > > You have lost the above 4 lines from every source file you updated. Thanks for catching it Phil. Updated. - PR Review Comment: https://git.openjdk.org/jdk/pull/18964#discussion_r1580239651
Re: RFR: JDK-8329004 : Update Libpng to 1.6.43 [v2]
> Libpng updated from 1.6.40 to 1.6.43 Harshitha Onkar has updated the pull request incrementally with one additional commit since the last revision: added missing 4 lines of header - Changes: - all: https://git.openjdk.org/jdk/pull/18964/files - new: https://git.openjdk.org/jdk/pull/18964/files/229f88f1..f9021883 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18964&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18964&range=00-01 Stats: 85 lines in 17 files changed: 85 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18964.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18964/head:pull/18964 PR: https://git.openjdk.org/jdk/pull/18964
Re: RFR: 8303689: javac -Xlint could/should report on "dangling" doc comments [v8]
> Please review the updates to support a proposed new
> `-Xlint:dangling-doc-comments` option.
>
> The work can be thought of as in 3 parts:
>
> 1. An update to the `javac` internal class `DeferredLintHandler` so that it
> is possible to specify the appropriately configured `Lint` object when it is
> time to consider whether to generate the diagnostic or not.
>
> 2. Updates to the `javac` front end to record "dangling docs comments" found
> near the beginning of a declaration, and to report them using an instance of
> `DeferredLintHandler`. This allows the warnings to be enabled or disabled
> using the standard mechanisms for `-Xlint` and `@SuppressWarnings`. The
> procedure for handling dangling doc comments is described in this comment in
> `JavacParser`.
>
> * Dangling documentation comments are handled as follows.
> * 1. {@code Scanner} adds all doc comments to a queue of
> * recent doc comments. The queue is flushed whenever
> * it is known that the recent doc comments should be
> * ignored and should not cause any warnings.
> * 2. The primary documentation comment is the one obtained
> * from the first token of any declaration.
> * (using {@code token.getDocComment()}.
> * 3. At the end of the "signature" of the declaration
> * (that is, before any initialization or body for the
> * declaration) any other "recent" comments are saved
> * in a map using the primary comment as a key,
> * using this method, {@code saveDanglingComments}.
> * 4. When the tree node for the declaration is finally
> * available, and the primary comment, if any,
> * is "attached", (in {@link #attach}) any related
> * dangling comments are also attached to the tree node
> * by registering them using the {@link #deferredLintHandler}.
> * 5. (Later) Warnings may be genereated for the dangling
> * comments, subject to the {@code -Xlint} and
> * {@code @SuppressWarnings}.
>
>
> 3. Updates to the make files to disable the warnings in modules for which
> the
> warning is generated. This is often because of the confusing use of `/**` to
> create box or other standout comments.
Jonathan Gibbons has updated the pull request with a new target base due to a
merge or a rebase. The pull request now contains 11 commits:
- Merge remote-tracking branch 'upstream/master' into
8303689.dangling-comments # Please enter a commit message to explain why this
merge is necessary, # especially if it merges an updated upstream into a topic
branch. # # Lines starting with '#' will be ignored, and an empty message
aborts # the
commit.
- Merge with upstream/master
- Merge remote-tracking branch 'upstream/master' into 8303689.dangling-comments
- Merge remote-tracking branch 'upstream/master' into 8303689.dangling-comments
- Merge with upstream/master
- update test
- improve handling of ignorable doc comments
suppress warning when building test code
- Merge remote-tracking branch 'upstream/master' into 8303689.dangling-comments
- call `saveDanglingDocComments` for local variable declarations
- adjust call for `saveDanglingDocComments` for enum members
- ... and 1 more: https://git.openjdk.org/jdk/compare/1c238d43...16a265a2
-
Changes: https://git.openjdk.org/jdk/pull/18527/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18527&range=07
Stats: 485 lines in 59 files changed: 387 ins; 3 del; 95 mod
Patch: https://git.openjdk.org/jdk/pull/18527.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/18527/head:pull/18527
PR: https://git.openjdk.org/jdk/pull/18527
Re: RFR: JDK-8329004 : Update Libpng to 1.6.43
On Thu, 25 Apr 2024 22:28:06 GMT, Harshitha Onkar wrote: > Libpng updated from 1.6.40 to 1.6.43 src/java.desktop/share/native/libsplashscreen/libpng/png.h line 30: > 28: * License version 2 only, as published by the Free Software Foundation. > 29: * However, the following notice accompanied the original version of this > 30: * file and, per its terms, should not be removed: You have lost the above 4 lines from every source file you updated. - PR Review Comment: https://git.openjdk.org/jdk/pull/18964#discussion_r1580217289
RFR: JDK-8329004 : Update Libpng to 1.6.43
Libpng updated from 1.6.40 to 1.6.43 - Commit messages: - .md file update - libpng update v1.6.43 Changes: https://git.openjdk.org/jdk/pull/18964/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18964&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8329004 Stats: 694 lines in 23 files changed: 258 ins; 205 del; 231 mod Patch: https://git.openjdk.org/jdk/pull/18964.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18964/head:pull/18964 PR: https://git.openjdk.org/jdk/pull/18964
Re: RFR: 8331011: [XWayland] TokenStorage fails under Security Manager [v2]
On Thu, 25 Apr 2024 17:22:48 GMT, Alexander Zvegintsev wrote: >> This fix adds missing doPrivileged calls in TokenStorage, which is used to >> help take screenshots in Wayland. > > Alexander Zvegintsev has updated the pull request incrementally with one > additional commit since the last revision: > > fix copyright years Marked as reviewed by prr (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/18950#pullrequestreview-2023323765
Re: Usage of iconv()
On 4/24/24 4:24 AM, Magnus Ihse Bursie wrote: That is a good question. libiconv is used only on macOS and AIX, for a few libraries, as you say. I just tried removing -liconv from the macOS dependencies and recompiled, just to see what would happen. There were three instances for macOS: libsplashscreen, libjdwp and libinstrument. libsplashscreen uses it in splashscreen_sys.m, where it is used to convert the jar file name. This is called from the launcher, in java.base/share/native/libjli/java.c libjdwp uses it in utf_util.c, where it is used to convert file name and command lines, iiuc. It is likely that we have similar (but better) ways to convert charsets elsewhere in our libraries that can be used instead of libiconv. I guess these are not the only two places where a filename must be converted from the platform charset to UTF8. So whatever replacement there might be, it needs to be something that is available very early in the life of the VM, in fact before there is a VM running. -phil.
Re: RFR: 8331011: [XWayland] TokenStorage fails under Security Manager [v2]
> This fix adds missing doPrivileged calls in TokenStorage, which is used to > help take screenshots in Wayland. Alexander Zvegintsev has updated the pull request incrementally with one additional commit since the last revision: fix copyright years - Changes: - all: https://git.openjdk.org/jdk/pull/18950/files - new: https://git.openjdk.org/jdk/pull/18950/files/5da7e9bf..73f21b6b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=18950&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=18950&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/18950.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18950/head:pull/18950 PR: https://git.openjdk.org/jdk/pull/18950
Re: RFR: 8331142: Add test for number of loader threads in BasicDirectoryModel
On Thu, 25 Apr 2024 16:37:39 GMT, Alexey Ivanov wrote: > This PR provides a regression test for > [JDK-8325179](https://bugs.openjdk.org/browse/JDK-8325179): _Race in > BasicDirectoryModel.validateFileCache_ reviewed in #18111. > > The test is inspired and based on `ConcurrentModification` that I wrote for > [JDK-8327137](https://bugs.openjdk.org/browse/JDK-8327137) / > [JDK-8323670](https://bugs.openjdk.org/browse/JDK-8323670). > > **How the test works** > > The test creates a temporary directory in the current directory and creates a > number of files in it. (The number of files is controlled by > `NUMBER_OF_THREADS` constant). Then the test creates `JFileChooser` in the > temporary directory. > > The test starts several scanner threads, the number is controlled by > `NUMBER_OF_THREADS`, which repeatedly call > `fileChooser.rescanCurrentDirectory()`. This results in calling > `BasicDirectoryModel.validateFileCache` which starts a background thread — > "Basic L&F File Loading Thread" — to enumerate the files. > > The test runner thread and scanner threads are synchronised with > `CyclicBarrier`, this ensures all the scanner threads start at the same time. > After a short delay, the runner thread takes a snapshot of live threads. > After a longer delay, the operation is repeated. See the `getThreadSnapshot` > method. (The number of snapshots is controlled by `SNAPSHOTS` constant.) > > The number of File Loading Threads in each snapshot is counted. There should > be no more than two threads. It is possible that thread two such threads > after JDK-8325179 is fixed: the existing thread is interrupted but hasn't > exited yet, and a new thread is already created. > > The test fails consistently without the fix for JDK-8325179. On Windows, the > output looks like this: > > > Number of snapshots: 20 > Number of snapshots where number of loader threads: > = 1: 0 > = 2: 0 > > 2: 20 > java.lang.RuntimeException: Detected 20 snapshots with several loading threads > at LoaderThreadCount.runTest(LoaderThreadCount.java:132) > at LoaderThreadCount.wrapper(LoaderThreadCount.java:72) > at java.base/java.lang.Thread.run(Thread.java:1583) > > > On Linux and macOS, there's more variation, for example I got the following > output on one of Linux systems: > > > Number of snapshots: 15 > Number of snapshots where number of loader threads: > = 1: 7 > = 2: 2 > > 2: 6 > > > The test passes on the builds where JDK-8325179 is present. Could you, @mrserb and @TejeshR13, take a look? You reviewed the test for [JDK-8323670](https://bugs.openjdk.org/browse/JDK-8323670) in #18109. Those who reviewed [JDK-8325179](https://bugs.openjdk.org/browse/JDK-8325179) in #18111 — @prrace and @turbanoff — could be interested too. - PR Comment: https://git.openjdk.org/jdk/pull/18957#issuecomment-2077726248
RFR: 8331142: Add test for number of loader threads in BasicDirectoryModel
This PR provides a regression test for [JDK-8325179](https://bugs.openjdk.org/browse/JDK-8325179): _Race in BasicDirectoryModel.validateFileCache_ reviewed in #18111. The test is inspired and based on `ConcurrentModification` that I wrote for [JDK-8327137](https://bugs.openjdk.org/browse/JDK-8327137) / [JDK-8323670](https://bugs.openjdk.org/browse/JDK-8323670). **How the test works** The test creates a temporary directory in the current directory and creates a number of files in it. (The number of files is controlled by `NUMBER_OF_THREADS` constant). Then the test creates `JFileChooser` in the temporary directory. The test starts several scanner threads, the number is controlled by `NUMBER_OF_THREADS`, which repeatedly call `fileChooser.rescanCurrentDirectory()`. This results in calling `BasicDirectoryModel.validateFileCache` which starts a background thread — "Basic L&F File Loading Thread" — to enumerate the files. The test runner thread and scanner threads are synchronised with `CyclicBarrier`, this ensures all the scanner threads start at the same time. After a short delay, the runner thread takes a snapshot of live threads. After a longer delay, the operation is repeated. See the `getThreadSnapshot` method. (The number of snapshots is controlled by `SNAPSHOTS` constant.) The number of File Loading Threads in each snapshot is counted. There should be no more than two threads. It is possible that thread two such threads after JDK-8325179 is fixed: the existing thread is interrupted but hasn't exited yet, and a new thread is already created. The test fails consistently without the fix for JDK-8325179. On Windows, the output looks like this: Number of snapshots: 20 Number of snapshots where number of loader threads: = 1: 0 = 2: 0 > 2: 20 java.lang.RuntimeException: Detected 20 snapshots with several loading threads at LoaderThreadCount.runTest(LoaderThreadCount.java:132) at LoaderThreadCount.wrapper(LoaderThreadCount.java:72) at java.base/java.lang.Thread.run(Thread.java:1583) On Linux and macOS, there's more variation, for example I got the following output on one of Linux systems: Number of snapshots: 15 Number of snapshots where number of loader threads: = 1: 7 = 2: 2 > 2: 6 The test passes on the builds where JDK-8325179 is present. - Commit messages: - Add copyright header; ensure headless mode (for macOS) - Merge master - Remove printing number of loader threads in each snapshot - Add comments to clarify how thread snapshots are captured - Use > 2 condition for error case - Add a test for JDK-8325179: count FilesLoader threads Changes: https://git.openjdk.org/jdk/pull/18957/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18957&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8331142 Stats: 263 lines in 1 file changed: 263 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18957.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18957/head:pull/18957 PR: https://git.openjdk.org/jdk/pull/18957
RFR: 8331011: [XWayland] TokenStorage fails under Security Manager
This fix adds missing doPrivileged calls in TokenStorage, which is used to help take screenshots in Wayland. - Commit messages: - initial Changes: https://git.openjdk.org/jdk/pull/18950/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=18950&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8331011 Stats: 46 lines in 1 file changed: 26 ins; 0 del; 20 mod Patch: https://git.openjdk.org/jdk/pull/18950.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18950/head:pull/18950 PR: https://git.openjdk.org/jdk/pull/18950
Re: RFR: 8327696: [TESTBUG] "javax/swing/JTable/KeyBoardNavigation/KeyBoardNavigation.java" test instruction needs to be corrected [v2]
On Tue, 23 Apr 2024 07:51:59 GMT, Tejesh R wrote: >> Instructions set has been updated as per OS specific. JTable keyboard >> navigation is tested in each OS and according it's current implementation >> the instructions has been updated (Few has been removed and few has been >> updated). >> PassFailJFrame.builder is used. > > Tejesh R has updated the pull request incrementally with one additional > commit since the last revision: > > Review updates Default time out may not be sufficient to test, can be increased. test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 198: > 196: final String WINDOWS_SPECIFIC = """ > 197: Tab, Shift-Tab - Navigate In. > 198: Return/Shift-Return - move focus one cell down/up. Suggestion: Return/Shift-Return - Move focus one cell down/up. For consistency please ensure each command action to start with either lower case or upper case. Same for F2, Esc commands etc. Check for Linux and Mac specific instructions also. test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 202: > 200: Up/Down Arrow - deselect current selection; move focus > one > 201: cell up/down > 202: Left/Right Arrow - deselect current selection; move focus Seems the instruction needs to modify. On press of Left/Right arrow, only focus shifted to one cell left or right but the current row selection remains same. Similar for Home/End as well, current selection remains as it is. test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 217: > 215: F2 - Allows editing in a cell containing information > without > 216: overwriting the information > 217: Esc - Resets the cell content back to the state it was > in Suggestion: Esc - Reset the cell content back to the state it was in. Minor suggestion, end with `.` for each statement else nothing. test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 220: > 218:before editing started > 219: Ctrl+A, Ctrl+/ - Select All > 220: Ctrl+\\ - De-select all Suggestion: Ctrl+\\ - deselect all test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 222: > 220: Ctrl+\\ - De-select all > 221: Shift-Up/Down Arrow - extend selection up/down one row > 222: Shift-Left/Right Arrow - extend selection left/right one No extended selection of columns only focus changed from one cell to another. test/jdk/javax/swing/JTable/KeyBoardNavigation.java line 226: > 224: Control-shift Up/Down Arrow - extend selection to > top/bottom > 225: of column > 226: Shift-Home/End - extend selection to left/right end of > row No extended selection only focus shifted to first and last column. - PR Review: https://git.openjdk.org/jdk/pull/18855#pullrequestreview-2022085343 PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579228099 PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579218399 PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579221288 PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579231419 PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579219557 PR Review Comment: https://git.openjdk.org/jdk/pull/18855#discussion_r1579220991
