Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base` [v2]

2024-04-19 Thread Jonathan Gibbons
On Fri, 19 Apr 2024 20:38:05 GMT, Naoto Sato wrote: > OK, fair enough. Approving for the `icu` part Thank you. - PR Comment: https://git.openjdk.org/jdk/pull/18846#issuecomment-2067280359

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base` [v2]

2024-04-19 Thread Naoto Sato
On Fri, 19 Apr 2024 19:21:13 GMT, Jonathan Gibbons wrote: >> Please review a set of updates to clean up use of `/**` comments in the >> vicinity of declarations. >> >> There are various categories of update: >> >> * "Box comments" beginning with `/**` >> * Misplaced doc comments before

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base` [v2]

2024-04-19 Thread Jonathan Gibbons
On Fri, 19 Apr 2024 19:47:20 GMT, Naoto Sato wrote: > Unless it is absolutely necessary, I would not fix comments in > `jdk.internal.icu` sources, as they are in the upstream code, and would like > to minimize the merging effort. @naotoj Given the policy and strong desire to compile

Re: RFR: 8323965: modify fix for 8317771 to remove reflection instantiation of the inner class

2024-04-19 Thread Chen Liang
On Fri, 19 Apr 2024 15:16:41 GMT, Artem Semenov wrote: > I replaced reflection with using an accessor > @azuev-java please review src/java.desktop/share/classes/sun/swing/SwingAccessor.java line 331: > 329: var access = accessibleJTreeNodeCreateAccessor; > 330: if (access ==

Re: RFR: 8322135: javax/swing/JTable/JTableScrollPrintTest.java & javax/swing/JTable/PrintAllPagesTest.java throws java.lang.InternalError: HTHEME is null

2024-04-19 Thread Alexey Ivanov
On Fri, 19 Apr 2024 19:19:26 GMT, Rajat Mahajan wrote: >> Getting a theme for particular dpi failed in windows L during print test. >> Before [JDK-8294427](https://bugs.openjdk.org/browse/JDK-8294427) fix, theme >> was independent of DPI. After the fix >>

Re: RFR: 8322135: javax/swing/JTable/JTableScrollPrintTest.java & javax/swing/JTable/PrintAllPagesTest.java throws java.lang.InternalError: HTHEME is null

2024-04-19 Thread Alexey Ivanov
On Wed, 10 Apr 2024 04:32:53 GMT, Tejesh R wrote: > Getting a theme for particular dpi failed in windows L during print test. > Before [JDK-8294427](https://bugs.openjdk.org/browse/JDK-8294427) fix, theme > was independent of DPI. After the fix >

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base` [v2]

2024-04-19 Thread Naoto Sato
On Fri, 19 Apr 2024 19:21:13 GMT, Jonathan Gibbons wrote: >> Please review a set of updates to clean up use of `/**` comments in the >> vicinity of declarations. >> >> There are various categories of update: >> >> * "Box comments" beginning with `/**` >> * Misplaced doc comments before

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Alexey Ivanov
On Fri, 19 Apr 2024 19:18:31 GMT, Jonathan Gibbons wrote: > We do not have an overall style guide. The conventional wisdom for editing > any existing file is to follow the style in that file, if such a style can be > discerned. That's what I do. I saw either style used in JDK. Yet I didn't

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base` [v2]

2024-04-19 Thread Alexey Ivanov
On Fri, 19 Apr 2024 19:21:13 GMT, Jonathan Gibbons wrote: >> Please review a set of updates to clean up use of `/**` comments in the >> vicinity of declarations. >> >> There are various categories of update: >> >> * "Box comments" beginning with `/**` >> * Misplaced doc comments before

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Jonathan Gibbons
On Fri, 19 Apr 2024 11:32:55 GMT, Pavel Rappo wrote: > This comment is not a review. I simply use the opportunity provided by this > PR to suggest that we stop making new `/** ... */` and separately fix old > jtreg comments like this: > > ``` > /** > * @test TestSmallHeap > * @bug 8067438

Re: RFR: 8322135: javax/swing/JTable/JTableScrollPrintTest.java & javax/swing/JTable/PrintAllPagesTest.java throws java.lang.InternalError: HTHEME is null

2024-04-19 Thread Harshitha Onkar
On Fri, 19 Apr 2024 05:07:19 GMT, Tejesh R wrote: > > @TejeshR13 Based on the PR description the source code fix looks correct. I > > was unable to test it on my local Win machine due to printer issues. The > > Print to PDF option isn't working too. But this is something to do with my > >

Re: RFR: 8322135: javax/swing/JTable/JTableScrollPrintTest.java & javax/swing/JTable/PrintAllPagesTest.java throws java.lang.InternalError: HTHEME is null

2024-04-19 Thread Rajat Mahajan
On Wed, 10 Apr 2024 04:32:53 GMT, Tejesh R wrote: > Getting a theme for particular dpi failed in windows L during print test. > Before [JDK-8294427](https://bugs.openjdk.org/browse/JDK-8294427) fix, theme > was independent of DPI. After the fix >

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base` [v2]

2024-04-19 Thread Jonathan Gibbons
On Fri, 19 Apr 2024 10:49:11 GMT, Alexey Ivanov wrote: >> Jonathan Gibbons has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update >> src/java.base/share/classes/sun/net/www/protocol/file/FileURLConnection.java >> >> Fix

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Jonathan Gibbons
On Thu, 18 Apr 2024 20:44:00 GMT, Jonathan Gibbons wrote: > Please review a set of updates to clean up use of `/**` comments in the > vicinity of declarations. > > There are various categories of update: > > * "Box comments" beginning with `/**` > * Misplaced doc comments before package or

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base` [v2]

2024-04-19 Thread Jonathan Gibbons
> Please review a set of updates to clean up use of `/**` comments in the > vicinity of declarations. > > There are various categories of update: > > * "Box comments" beginning with `/**` > * Misplaced doc comments before package or import statements > * Misplaced doc comments after the

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Jonathan Gibbons
On Fri, 19 Apr 2024 10:44:27 GMT, Alexey Ivanov wrote: >> Please review a set of updates to clean up use of `/**` comments in the >> vicinity of declarations. >> >> There are various categories of update: >> >> * "Box comments" beginning with `/**` >> * Misplaced doc comments before package

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Jonathan Gibbons
On Fri, 19 Apr 2024 10:53:11 GMT, Alexey Ivanov wrote: >> Please review a set of updates to clean up use of `/**` comments in the >> vicinity of declarations. >> >> There are various categories of update: >> >> * "Box comments" beginning with `/**` >> * Misplaced doc comments before package

Re: RFR: 8322135: javax/swing/JTable/JTableScrollPrintTest.java & javax/swing/JTable/PrintAllPagesTest.java throws java.lang.InternalError: HTHEME is null

2024-04-19 Thread Phil Race
On Wed, 10 Apr 2024 04:32:53 GMT, Tejesh R wrote: > Getting a theme for particular dpi failed in windows L during print test. > Before [JDK-8294427](https://bugs.openjdk.org/browse/JDK-8294427) fix, theme > was independent of DPI. After the fix >

Re: RFR: 8322135: javax/swing/JTable/JTableScrollPrintTest.java & javax/swing/JTable/PrintAllPagesTest.java throws java.lang.InternalError: HTHEME is null

2024-04-19 Thread Victor Dyakov
On Wed, 10 Apr 2024 04:32:53 GMT, Tejesh R wrote: > Getting a theme for particular dpi failed in windows L during print test. > Before [JDK-8294427](https://bugs.openjdk.org/browse/JDK-8294427) fix, theme > was independent of DPI. After the fix >

RFR: 8323965: modify fix for 8317771 to remove reflection instantiation of the inner class

2024-04-19 Thread Artem Semenov
I replaced reflection with using an accessor @azuev-java please review - Commit messages: - 8323965 modify fix for 8317771 to remove reflection instantiation of the inner class Changes: https://git.openjdk.org/jdk/pull/18867/files Webrev:

Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled [v3]

2024-04-19 Thread Alexey Ivanov
On Fri, 19 Apr 2024 14:53:09 GMT, Renjith Kannath Pariyangad wrote: >> Hi Reviewers, >> >> Added pageloader cancel before new page creation along with code >> restructuring. Moved all page loading calls inside synchronize to make it >> thread safe. >> >> Regards, >> Renjith. > > Renjith

Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled [v3]

2024-04-19 Thread Renjith Kannath Pariyangad
> Hi Reviewers, > > Added pageloader cancel before new page creation along with code > restructuring. Moved all page loading calls inside synchronize to make it > thread safe. > > Regards, > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional

Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled [v2]

2024-04-19 Thread Alexey Ivanov
On Fri, 19 Apr 2024 12:30:26 GMT, Renjith Kannath Pariyangad wrote: >> Hi Reviewers, >> >> Added pageloader cancel before new page creation along with code >> restructuring. Moved all page loading calls inside synchronize to make it >> thread safe. >> >> Regards, >> Renjith. > > Renjith

Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled [v2]

2024-04-19 Thread Renjith Kannath Pariyangad
On Thu, 18 Apr 2024 16:41:34 GMT, Alexey Ivanov wrote: >> Renjith Kannath Pariyangad has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Suggesions updated > > src/java.desktop/share/classes/javax/swing/JEditorPane.java line 497: > >> 495:

Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled [v2]

2024-04-19 Thread Renjith Kannath Pariyangad
> Hi Reviewers, > > Added pageloader cancel before new page creation along with code > restructuring. Moved all page loading calls inside synchronize to make it > thread safe. > > Regards, > Renjith. Renjith Kannath Pariyangad has updated the pull request incrementally with one additional

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Pavel Rappo
On Thu, 18 Apr 2024 20:44:00 GMT, Jonathan Gibbons wrote: > Please review a set of updates to clean up use of `/**` comments in the > vicinity of declarations. > > There are various categories of update: > > * "Box comments" beginning with `/**` > * Misplaced doc comments before package or

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Daniel Fuchs
On Thu, 18 Apr 2024 20:44:00 GMT, Jonathan Gibbons wrote: > Please review a set of updates to clean up use of `/**` comments in the > vicinity of declarations. > > There are various categories of update: > > * "Box comments" beginning with `/**` > * Misplaced doc comments before package or

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Alexey Ivanov
On Thu, 18 Apr 2024 20:44:00 GMT, Jonathan Gibbons wrote: > Please review a set of updates to clean up use of `/**` comments in the > vicinity of declarations. > > There are various categories of update: > > * "Box comments" beginning with `/**` > * Misplaced doc comments before package or

Re: RFR: 8330178: Clean up non-standard use of /** comments in `java.base`

2024-04-19 Thread Alexey Ivanov
On Thu, 18 Apr 2024 20:44:00 GMT, Jonathan Gibbons wrote: > Please review a set of updates to clean up use of `/**` comments in the > vicinity of declarations. > > There are various categories of update: > > * "Box comments" beginning with `/**` > * Misplaced doc comments before package or

Re: RFR: 8328977 : JEditorPane.setPage not thread-safe, pageLoader not cancelled

2024-04-19 Thread Alexey Ivanov
On Fri, 19 Apr 2024 05:10:46 GMT, Tejesh R wrote: > I wasn't able to make out the changes with comparison since restructuring is > also done here. Just like you are explaining here would help in understanding > the changes for review. Putting all access to `pageLoader` into a `synchronized`

RFR: 8327696: [TESTBUG] "javax/swing/JTable/KeyBoardNavigation/KeyBoardNavigation.java" test instruction needs to be corrected

2024-04-19 Thread Tejesh R
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. - Commit messages: -

Integrated: 8310072: JComboBox/DisabledComboBoxFontTestAuto: Enabled and disabled ComboBox does not match in these LAFs: GTK+

2024-04-19 Thread Abhishek Kumar
On Fri, 5 Apr 2024 06:54:46 GMT, Abhishek Kumar wrote: > Test was failing on GTK and Windows LAF due to pixel color mismatch. Th > reason behind this issue was the size of image which is different and that > results in incorrect pixel comparison. Fix is to ensure that correct pixel is >

Re: RFR: 8310072: JComboBox/DisabledComboBoxFontTestAuto: Enabled and disabled ComboBox does not match in these LAFs: GTK+ [v5]

2024-04-19 Thread Abhishek Kumar
> Test was failing on GTK and Windows LAF due to pixel color mismatch. Th > reason behind this issue was the size of image which is different and that > results in incorrect pixel comparison. Fix is to ensure that correct pixel is > matched and the pixel color should remain within tolerance. >