Re: RFR: 8297481: Create a regression test for JDK-4424517
On Wed, 23 Nov 2022 08:57:23 GMT, Srinivas Mandalika wrote: > 8297481: Create a regression test for JDK-4424517 Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11313
Re: RFR: 8296905: Replace the native LCMS#getProfileID() method with the accessor [v3]
On Fri, 25 Nov 2022 02:21:09 GMT, SWinxy wrote:
>> Sergey Bylokhov has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains four additional
>> commits since the last revision:
>>
>> - Merge branch 'openjdk:master' into JDK-8296905
>> - Update AWTAccessor.java
>> - Merge remote-tracking branch 'upstream/master' into JDK-8296905
>> - 8296905: Replace the native LCMS#getProfileID() method with the accessor
>
> src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMS.java line 59:
>
>> 57: if (p instanceof LCMSProfile) {
>> 58: return (LCMSProfile)p;
>> 59: }
>
> Would be a good time to
> Suggestion:
>
> if (p instanceof LCMSProfile profile) {
> return profile;
> }
That change is possible but it will be better to do it as a separate cleanup.
-
PR: https://git.openjdk.org/jdk/pull/6
Re: RFR: 8296905: Replace the native LCMS#getProfileID() method with the accessor [v3]
On Fri, 25 Nov 2022 01:34:26 GMT, Sergey Bylokhov wrote:
>> The native method used to access the private method in the `ICC_Profile`
>> class is replaced by the accessor.
>
> Sergey Bylokhov has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains four additional
> commits since the last revision:
>
> - Merge branch 'openjdk:master' into JDK-8296905
> - Update AWTAccessor.java
> - Merge remote-tracking branch 'upstream/master' into JDK-8296905
> - 8296905: Replace the native LCMS#getProfileID() method with the accessor
src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMS.java line 59:
> 57: if (p instanceof LCMSProfile) {
> 58: return (LCMSProfile)p;
> 59: }
Would be a good time to
Suggestion:
if (p instanceof LCMSProfile profile) {
return profile;
}
-
PR: https://git.openjdk.org/jdk/pull/6
Re: RFR: 8296905: Replace the native LCMS#getProfileID() method with the accessor [v3]
> The native method used to access the private method in the `ICC_Profile` > class is replaced by the accessor. Sergey Bylokhov has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Merge branch 'openjdk:master' into JDK-8296905 - Update AWTAccessor.java - Merge remote-tracking branch 'upstream/master' into JDK-8296905 - 8296905: Replace the native LCMS#getProfileID() method with the accessor - Changes: - all: https://git.openjdk.org/jdk/pull/6/files - new: https://git.openjdk.org/jdk/pull/6/files/f7b74481..9e7819fb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=6&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=6&range=01-02 Stats: 26596 lines in 411 files changed: 11421 ins; 4961 del; 10214 mod Patch: https://git.openjdk.org/jdk/pull/6.diff Fetch: git fetch https://git.openjdk.org/jdk pull/6/head:pull/6 PR: https://git.openjdk.org/jdk/pull/6
RFR: 8296660: Swing HTML table with omitted closing tags misparsed
This is in continuation with https://github.com/openjdk/jdk/commit/7133fc93e168f3671d048b2ae654f84ec289b98d fix done for [JDK-7172359](https://bugs.openjdk.org/browse/JDK-7172359) issue where fix was done to rectify invalid tag causing StackOverflowError but it caused alignment issue if the closing tags are optional, so the fix is modified to check for optional closing tag in which case dont return false for legalElementContext() JDK-7172359 fix and other CI regression tests are fine. - Commit messages: - Fix - Fix - 8296660: Swing HTML table with omitted closing tags misparsed Changes: https://git.openjdk.org/jdk/pull/11355/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11355&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8296660 Stats: 74 lines in 2 files changed: 73 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/11355.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11355/head:pull/11355 PR: https://git.openjdk.org/jdk/pull/11355
Re: RFR: 8078471: The BACKSPACE key doesn't work with the special options"-client -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel" [v2]
On Thu, 24 Nov 2022 13:29:12 GMT, Prasanta Sadhukhan wrote: > This mapping ""BACK_SPACE", "Go Up"," I guess has nothing to do with > squarebutton property..Probably you are confusing it with your other fix? Yeah, I am sorry. I will check in Nimbus then how it's happening. > You probably can do requestFocusInWindow to proper component...you can try to > find the who has the focus by getFocusOwner() when you try to do manually > BACKSPACE and try to set to that in automated test... Ok. - PR: https://git.openjdk.org/jdk/pull/11291
Re: RFR: 8078471: The BACKSPACE key doesn't work with the special options"-client -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel" [v2]
On Thu, 24 Nov 2022 08:57:45 GMT, Abhishek Kumar wrote: >> The Backspace key doesn't lead to parent directory of current directory for >> JFileChooser in GTK LAF. >> Added the lazy input value in `GTKLookandFeel` and fix is working fine. >> >> Implemented a manual test to check BackSpace behavior for all installed LAF >> in Linux platform. >> The test mentioned in >> [JDK-8078471](https://bugs.openjdk.org/browse/JDK-8078471) >> `javax/swing/JFileChooser/4150029/bug4150029.html` also working fine with >> GTK option (-client >> -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel). > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Backspace at root level handled `>` There is a mapping for squarebutton in skin.laf ``. > Setting it true is having similar effect in Nimbus LAF also. So, I think in > Nimbus LAF the squarebutton property is set through skin.laf file. This mapping ""BACK_SPACE", "Go Up"," I guess has nothing to do with squarebutton property..Probably you are confusing it with your other fix? > I tried implementing an automated test case but in GTK LAF the cursor is > focused on textfield and the backspace event is not triggered to change > directory. You probably can do `requestFocusInWindow `to proper component...you can try to find the who has the focus by `getFocusOwner`() when you try to do manually BACKSPACE and try to set to that in automated test... - PR: https://git.openjdk.org/jdk/pull/11291
Re: RFR: 8078471: The BACKSPACE key doesn't work with the special options"-client -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel" [v2]
On Thu, 24 Nov 2022 13:00:52 GMT, Prasanta Sadhukhan wrote: > I wonder how it works in Nimbus as this mapping is not there in > Synth/NimbusLookAndFeel and still it works and both GTK/Nimbus extends Synth. > Can you please check? There is a mapping for squarebutton in skin.laf ``. Setting it true is having similar effect in Nimbus LAF also. So, I think in Nimbus LAF the squarebutton property is set through skin.laf file. > You have a similar fix for GTK > https://github.com/openjdk/jdk/commit/9d0009e92b790b43153e3db353db775e6ff731cb Can it be reused in this case also? Don't think it can be reused in this case as the event is triggered from either double click or keyboard (Enter) on "../" directory present in JList. I will verify this once. > Also, is it not possible to create automated test, like calling > JFileChooser.getCurrentDirectory() and fail if it's not the parent directory > (or if it still remains in present directory) after BACKSPACE is pressed? I tried implementing an automated test case but in GTK LAF the cursor is focused on textfield and the backspace event is not triggered to change directory. - PR: https://git.openjdk.org/jdk/pull/11291
Re: RFR: 8078471: The BACKSPACE key doesn't work with the special options"-client -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel" [v2]
On Thu, 24 Nov 2022 08:57:45 GMT, Abhishek Kumar wrote: >> The Backspace key doesn't lead to parent directory of current directory for >> JFileChooser in GTK LAF. >> Added the lazy input value in `GTKLookandFeel` and fix is working fine. >> >> Implemented a manual test to check BackSpace behavior for all installed LAF >> in Linux platform. >> The test mentioned in >> [JDK-8078471](https://bugs.openjdk.org/browse/JDK-8078471) >> `javax/swing/JFileChooser/4150029/bug4150029.html` also working fine with >> GTK option (-client >> -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel). > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Backspace at root level handled I wonder how it works in Nimbus as this mapping is not there in Synth/NimbusLookAndFeel and still it works and both GTK/Nimbus extends Synth. Can you please check? > If JFileChooser current directory is set to root directory then pressing > BackSpace key changes to user's home directory in GTK LAF which is incorrect. > It should remain at root directory. You have a similar fix for GTK https://github.com/openjdk/jdk/commit/9d0009e92b790b43153e3db353db775e6ff731cb Can it be reused in this case also? Also, is it not possible to create automated test, like calling JFileChooser.getCurrentDirectory() and fail if it's not the parent directory (or if it still remains in present directory) after BACKSPACE is pressed? - PR: https://git.openjdk.org/jdk/pull/11291
Re: RFR: 8078471: The BACKSPACE key doesn't work with the special options"-client -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel" [v2]
On Thu, 24 Nov 2022 08:57:45 GMT, Abhishek Kumar wrote: >> The Backspace key doesn't lead to parent directory of current directory for >> JFileChooser in GTK LAF. >> Added the lazy input value in `GTKLookandFeel` and fix is working fine. >> >> Implemented a manual test to check BackSpace behavior for all installed LAF >> in Linux platform. >> The test mentioned in >> [JDK-8078471](https://bugs.openjdk.org/browse/JDK-8078471) >> `javax/swing/JFileChooser/4150029/bug4150029.html` also working fine with >> GTK option (-client >> -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel). > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Backspace at root level handled Run the CI clienlibs job with or without fix. There were few failures observed. 2 of them are common in both and 1 failure is observed in particular machine. Link is added in JBS. - PR: https://git.openjdk.org/jdk/pull/11291
Re: RFR: 8296546: Add @spec tags to API [v3]
On Wed, 23 Nov 2022 18:57:03 GMT, Jonathan Gibbons wrote: >> Please review a "somewhat automated" change to insert `@spec` tags into doc >> comments, as appropriate, to leverage the recent new javadoc feature to >> generate a new page listing the references to all external specifications >> listed in the `@spec` tags. >> >> "Somewhat automated" means that I wrote and used a temporary utility to scan >> doc comments looking for HTML links to selected sites, such as `ietf.org`, >> `unicode.org`, `w3.org`. These links may be in the main description of a doc >> comment, or in `@see` tags. For each link, the URL is examined, and >> "normalized", and inserted into the doc comment with a new `@spec` tag, >> giving the link and tile for the spec. >> >> "Normalized" means... >> * Use `https:` where possible (includes pretty much all cases) >> * Use a single consistent host name for all URLs coming from the same spec >> site (i.e. don't use different aliases for the same site) >> * Point to the root page of a multi-page spec >> * Use a consistent form of the spec, preferring HTML over plain text where >> both are available (this mostly applies to IETF specs) >> >> In addition, a "standard" title is determined for all specs, determined >> either from the content of the (main) spec page or from site index pages. >> >> The net effect is (or should be) that **all** the changes are to just >> **add** new `@spec` tags, based on the links found in each doc comment. >> There should be no other changes to the doc comments, or to the >> implementation of any classes and interfaces. >> >> That being said, the utility I wrote does have additional abilities, to >> update the links that it finds (e.g. changing to use `https:` etc,) but >> those features are _not_ being used here, but could be used in followup PRs >> if component teams so desired. I did notice while working on this overall >> feature that many of our links do point to "outdated" pages, some with >> eye-catching notices declaring that the spec has been superseded. >> Determining how, when and where to update such links is beyond the scope of >> this PR. >> >> Going forward, it is to be hoped that component teams will maintain the >> underlying links, and the URLs in `@spec` tags, such that if references to >> external specifications are updated, this will include updating the `@spec` >> tags. >> >> To see the effect of all these new `@spec` tags, see >> http://cr.openjdk.java.net/~jjg/8296546/api.00/ >> >> In particular, see the new [External >> Specifications](http://cr.openjdk.java.net/~jjg/8296546/api.00/external-specs.html) >> page, which you can also find via the new link near the top of the >> [Index](http://cr.openjdk.java.net/~jjg/8296546/api.00/index-files/index-1.html) >> pages. > > Jonathan Gibbons has updated the pull request incrementally with one > additional commit since the last revision: > > Remove updates from unexported files src/java.se/share/classes/module-info.java line 39: > 37: * > 38: * @spec jdwp/jdwp-spec.html Java Debug Wire Protocol > 39: * @spec jni/index.html Java Native Interface Specification One thing that that bothers me a bit here is that the JNI and JDWP specs will be listed as "External Specifications" in the generated javadoc. This heading is appropriate for RFCs and other standards that we reference but seems misleading for specifications that are part of Java SE. Has this come up already? - PR: https://git.openjdk.org/jdk/pull/11073
Re: RFR: 8297095: Write a test to determine the quit request of the application
On Wed, 23 Nov 2022 18:29:50 GMT, Harshitha Onkar wrote: >> This testcase will >> 1) Verify the handleQuitRequestWith() method of QuitHandler interface. >> 2) Check that the response is passed to the handler by means of quit >> notification generated by a key combination of META+Q (Mac) or >> ALT+F4(Windows & Linux). >> 3) Confirm that the active window is closed. >> >> Testing: >> Tested using Mach5(20 times per platform) in Mac OS, Linux and Windows and >> got all pass. > > test/jdk/java/awt/Desktop/ActiveWindowCloseTest.java line 2: > >> 1: >> 2: /* Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights >> reserved. > > If this is a newly created test, then the copyright should contain only the > present year. I have added the original date based on the above comment that’s we get during the PR review below https://github.com/openjdk/jdk/pull/7512#issuecomment-1075171066 - PR: https://git.openjdk.org/jdk/pull/11328
Re: RFR: JDK-8297480: GetPrimitiveArrayCritical in imageioJPEG misses result - NULL check
On Wed, 23 Nov 2022 08:18:32 GMT, Matthias Baesken wrote: > Seems there is a remaining GetPrimitiveArrayCritical in imageioJPEG that > misses a result - NULL check, this should be added. some tests are failing - PR: https://git.openjdk.org/jdk/pull/11312
Re: RFR: 8078471: The BACKSPACE key doesn't work with the special options"-client -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel" [v2]
On Thu, 24 Nov 2022 08:57:45 GMT, Abhishek Kumar wrote: >> The Backspace key doesn't lead to parent directory of current directory for >> JFileChooser in GTK LAF. >> Added the lazy input value in `GTKLookandFeel` and fix is working fine. >> >> Implemented a manual test to check BackSpace behavior for all installed LAF >> in Linux platform. >> The test mentioned in >> [JDK-8078471](https://bugs.openjdk.org/browse/JDK-8078471) >> `javax/swing/JFileChooser/4150029/bug4150029.html` also working fine with >> GTK option (-client >> -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel). > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Backspace at root level handled Updated to handle backspace key at root directory. - PR: https://git.openjdk.org/jdk/pull/11291
Re: RFR: 6972078: Can not select single directory with GTKLookAndFeel [v6]
On Tue, 8 Nov 2022 07:06:27 GMT, Abhishek Kumar wrote: >> While using a JFileChooser with the file selection mode >> FILES_AND_DIRECTORIES and multiSelection enabled, it is impossible to select >> a single directory in GTK LAF. >> >> The condition check has been modified when multiselection is enabled and >> user has selected single directory. >> After the fix the behavior of JFileChooser is similar to other LAFs. >> >> An automatic test case >> `test/jdk/com/sun/java/swing/plaf/gtk/TestFileChooserSingleDirectorySelection.java` >> is implemented and checked in CI pipeline. Link is attached in JBS. > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > Review comment fix lgtm - PR: https://git.openjdk.org/jdk/pull/10866
Re: RFR: 8078471: The BACKSPACE key doesn't work with the special options"-client -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel" [v2]
> The Backspace key doesn't lead to parent directory of current directory for > JFileChooser in GTK LAF. > Added the lazy input value in `GTKLookandFeel` and fix is working fine. > > Implemented a manual test to check BackSpace behavior for all installed LAF > in Linux platform. > The test mentioned in > [JDK-8078471](https://bugs.openjdk.org/browse/JDK-8078471) > `javax/swing/JFileChooser/4150029/bug4150029.html` also working fine with GTK > option (-client > -Dswing.defaultlaf=com.sun.java.swing.plaf.gtk.GTKLookAndFeel). Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision: Backspace at root level handled - Changes: - all: https://git.openjdk.org/jdk/pull/11291/files - new: https://git.openjdk.org/jdk/pull/11291/files/634c507d..1fcdc76c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=11291&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=11291&range=00-01 Stats: 10 lines in 2 files changed: 5 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/11291.diff Fetch: git fetch https://git.openjdk.org/jdk pull/11291/head:pull/11291 PR: https://git.openjdk.org/jdk/pull/11291
Re: RFR: JDK-8297480: GetPrimitiveArrayCritical in imageioJPEG misses result - NULL check
On Wed, 23 Nov 2022 08:18:32 GMT, Matthias Baesken wrote: > Seems there is a remaining GetPrimitiveArrayCritical in imageioJPEG that > misses a result - NULL check, this should be added. Thanks for the review ! - PR: https://git.openjdk.org/jdk/pull/11312
Integrated: JDK-8297480: GetPrimitiveArrayCritical in imageioJPEG misses result - NULL check
On Wed, 23 Nov 2022 08:18:32 GMT, Matthias Baesken wrote: > Seems there is a remaining GetPrimitiveArrayCritical in imageioJPEG that > misses a result - NULL check, this should be added. This pull request has now been integrated. Changeset: 2f8a5c2e Author:Matthias Baesken URL: https://git.openjdk.org/jdk/commit/2f8a5c2eca0dc3dad08b7b2c33394ac214907008 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8297480: GetPrimitiveArrayCritical in imageioJPEG misses result - NULL check Reviewed-by: jdv - PR: https://git.openjdk.org/jdk/pull/11312
