Integrated: 8264102: JTable Keyboards Navigation differs with Test Instructions.
On Wed, 3 Jan 2024 04:20:44 GMT, Tejesh R wrote: > Updated the actions for "ctrl shift DOWN" to "`selectLastRowExtendSelection`" > and "ctrl shift UP", "`selectFirstRowExtendSelection`" in Basic and other > Look and feel. Tested in CI and no regressions found. > Test to be used - > [javax/swing/JTable/KeyBoardNavigation/KeyBoardNavigation.java](https://github.com/openjdk/jdk/blob/master/test/jdk/javax/swing/JTable/KeyBoardNavigation/KeyBoardNavigation.java) This pull request has now been integrated. Changeset: 2b7fc050 Author:Tejesh R URL: https://git.openjdk.org/jdk/commit/2b7fc0506ab37f1ec1e63542fb0dcd710c33ef93 Stats: 468 lines in 12 files changed: 238 ins; 213 del; 17 mod 8264102: JTable Keyboards Navigation differs with Test Instructions. Reviewed-by: psadhukhan, abhiscxk - PR: https://git.openjdk.org/jdk/pull/17236
Re: RFR: 8295804: javax/swing/JFileChooser/JFileChooserSetLocationTest.java failed with "setLocation() is not working properly" [v2]
> The issue is that the doc area (in respect to the screen height which is > 768px) which is at the bottom was causing the `JFileChooser `to be placed > slightly above the set location. Was able to reproduce in local machine with > reference to the failure image provided in the CI logs. The suggested fix is > to place the main Frame slightly above the center of the screen than setting > at the center of the screen. Several CI runs were made and no issue found. Tejesh R has updated the pull request incrementally with one additional commit since the last revision: Review fix - Changes: - all: https://git.openjdk.org/jdk/pull/17364/files - new: https://git.openjdk.org/jdk/pull/17364/files/77926ea3..b6a099e9 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17364=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17364=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17364.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17364/head:pull/17364 PR: https://git.openjdk.org/jdk/pull/17364
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix
On Wed, 10 Jan 2024 19:50:45 GMT, Sergey Bylokhov wrote: >> Can this patch be covered by the new test? > >>I'm not sure… We want the filter to take another path, there could be a list >>of filters applied, if I understand @mrserb correctly. Sergey could be able >>to provide a more detailed guidance. > > The current test validates two code paths: > - wrapper>icc_color_space->wrapper > - icc_color_space->icc_color_space->icc_color_space > > The color space in the middle is always icc_color_space: > https://github.com/openjdk/jdk/pull/16895/files#diff-70b19b2642d6d3f44904de8b6eb2993e1c97320e3476898c4372db364c4288b7R130 > > If we will use the wrapper for the middle as well we will cover this code > path: > https://github.com/openjdk/jdk/pull/16895/files#diff-e3d6eea060882cab00827c00e1a83b0e0a5b2a31fa9a9aa2122841bbd57c4a6dL853 @mrserb , Did you mean to use the wrapper for the middle like `ColorSpace mid = createCS(ColorSpaceSelector.WRAPPED_PYCC);` , So we can achieve : **Current** **New** wrapper->icc_color_space->wrapper> wrapper->wrapper->wrapper icc_color_space->icc_color_space->icc_color_space > icc_color_space->wrapper->icc_color_space Correct me if I am wrong - PR Comment: https://git.openjdk.org/jdk/pull/16895#issuecomment-1886405188
Re: RFR: 8295804: javax/swing/JFileChooser/JFileChooserSetLocationTest.java failed with "setLocation() is not working properly"
On Thu, 11 Jan 2024 04:48:23 GMT, Tejesh R wrote: > The issue is that the doc area (in respect to the screen height which is > 768px) which is at the bottom was causing the `JFileChooser `to be placed > slightly above the set location. Was able to reproduce in local machine with > reference to the failure image provided in the CI logs. The suggested fix is > to place the main Frame slightly above the center of the screen than setting > at the center of the screen. Several CI runs were made and no issue found. test/jdk/javax/swing/JFileChooser/JFileChooserSetLocationTest.java line 217: > 215: int screenWidth = (int) screenSize.getWidth() / 2; > 216: int screenHeight = (int) screenSize.getHeight() / 2; > 217: frame = new JFrame(); Frame should have title. - PR Review Comment: https://git.openjdk.org/jdk/pull/17364#discussion_r1448363735
Re: RFR: 8295804: javax/swing/JFileChooser/JFileChooserSetLocationTest.java failed with "setLocation() is not working properly"
On Thu, 11 Jan 2024 06:14:48 GMT, Abhishek Kumar wrote: > Does the test fails only on the system which is 768px in height or in any > other screen size also? Not only on 768, whichever size isn't enough for the FileChooser to show up fully it'll fail. I was able to check till 720px in local machine where it fails. After fix I tested in all C machines and it is green. - PR Comment: https://git.openjdk.org/jdk/pull/17364#issuecomment-1886373685
Re: RFR: 8295804: javax/swing/JFileChooser/JFileChooserSetLocationTest.java failed with "setLocation() is not working properly"
On Thu, 11 Jan 2024 04:48:23 GMT, Tejesh R wrote: > The issue is that the doc area (in respect to the screen height which is > 768px) which is at the bottom was causing the `JFileChooser `to be placed > slightly above the set location. Was able to reproduce in local machine with > reference to the failure image provided in the CI logs. The suggested fix is > to place the main Frame slightly above the center of the screen than setting > at the center of the screen. Several CI runs were made and no issue found. Does the test fails only on the system which is 768px in height or in any other screen size also? - PR Comment: https://git.openjdk.org/jdk/pull/17364#issuecomment-1886362201
Re: RFR: 8295804: javax/swing/JFileChooser/JFileChooserSetLocationTest.java failed with "setLocation() is not working properly"
On Thu, 11 Jan 2024 05:31:50 GMT, Abhishek Kumar wrote: > > The issue is that the doc area (`in respect to the screen width which is > > 768px`) which is at the bottom was causing the JFileChooser to be placed > > slightly above the set location. > > Is it with respect to height ? Yeah, corrected now. - PR Comment: https://git.openjdk.org/jdk/pull/17364#issuecomment-1886326932
Re: RFR: 8295804: javax/swing/JFileChooser/JFileChooserSetLocationTest.java failed with "setLocation() is not working properly"
On Thu, 11 Jan 2024 04:48:23 GMT, Tejesh R wrote: > The issue is that the doc area (`in respect to the screen width which is > 768px`) which is at the bottom was causing the JFileChooser to be placed > slightly above the set location. Is it with respect to height ? - PR Comment: https://git.openjdk.org/jdk/pull/17364#issuecomment-1886273346
Re: RFR: JDK-8312518 : [macos13] setFullScreenWindow() shows black screen on macOS 13 & above
On Wed, 10 Jan 2024 21:35:57 GMT, Harshitha Onkar wrote:
> A black screen is seen on newer versions of macOS (13.3 & above) when a
> window is set to full-screen using `setFullScreenWindow()`. The root cause
> was narrowed down to the shield level of the full-screen window vs the shield
> level of the captured display.
>
> Following solutions were explored -
>
> 1. Setting `kCGMaximumWindowLevelKey` as the shield level for the full screen
> window. But setting the fullscreen window to maximum available window level
> might cause z-order issues when other popup/screen savers are involved.
>
> int shieldLevel = CGWindowLevelForKey(kCGMaximumWindowLevelKey);
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: shieldLevel];
>
> 2. Raise the window's level slightly (shieldLevel + 1) above the system
> shield window.
>
> int shieldLevel = CGShieldingWindowLevel();
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: (shieldLevel + 1)];
>
> 3. Keeping the shielding level as-is and bringing the window to the
> foreground after display is captured. The 3rd approach **(also the one Apple
> recommends)** ensures that the full screen window has focus as well as being
> visible and also maintains the correct z-order. This solution works as
> expected on older (< 13.3) and newer versions (13.3 & above) of macOS.
>
> if (CGDisplayCapture(aID) == kCGErrorSuccess) {
> ...
> ...
> int shieldLevel = CGShieldingWindowLevel();
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: shieldLevel];
> [nsWindow makeKeyAndOrderFront: nil];
> }
Looks good to me. Tested and its working as expected.
-
Marked as reviewed by tr (Committer).
PR Review: https://git.openjdk.org/jdk/pull/17358#pullrequestreview-1814687969
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v4]
On Wed, 10 Jan 2024 12:31:57 GMT, Prasanta Sadhukhan wrote: >> I tried with this approach first but images didn't render at all. > > I guess those might be null at class initialisation stage so it didn't > render..you can try > > expandedIconWrapper = new IconWrapper(expandedIcon); > collapsedIconWrappr - new IconWrapper(collapsedICon); > > in updateStyle() where it sets > > setExpandedIcon(style.getIcon(context, "Tree.expandedIcon")); > setCollapsedIcon(style.getIcon(context, "Tree.collapsedIcon")); Updated. - PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1448302768
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v5]
> The collapsed icon for JTree is not painted using `Icon.paintIcon(Component > c, Graphics g, int x, int y)` in GTK LAF. The collapsed icon is returned from > BasicTreeUI class which doesn't contain any icon image whereas the expanded > icon is returned from SynthTreeUI class and expanded icon is rendered > correctly. > The proposed fix is to return collapsed icon as an object of collapsed icon > wrapper which implements synthIcon and is similar to the expanded icon > implementation. > > Test mentioned in JBS is an applet based which is converted to main based now > and extended for all installed LAFs on the system. > > No regression caused with the fix, link is attached in JBS . Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision: Review comment fix and move file outside folder - Changes: - all: https://git.openjdk.org/jdk/pull/17294/files - new: https://git.openjdk.org/jdk/pull/17294/files/2fa05f9f..17d9bdc2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17294=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17294=03-04 Stats: 165 lines in 2 files changed: 5 ins; 152 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/17294.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17294/head:pull/17294 PR: https://git.openjdk.org/jdk/pull/17294
RFR: 8295804: javax/swing/JFileChooser/JFileChooserSetLocationTest.java failed with "setLocation() is not working properly"
The issue is that the doc area (in respect to the screen width which is 768px) which is at the bottom was causing the `JFileChooser `to be placed slightly above the set location. Was able to reproduce in local machine with reference to the failure image provided in the CI logs. The suggested fix is to place the main Frame slightly above the center of the screen than setting at the center of the screen. Several CI runs were made and no issue found. - Commit messages: - Fix Changes: https://git.openjdk.org/jdk/pull/17364/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17364=00 Issue: https://bugs.openjdk.org/browse/JDK-8295804 Stats: 9 lines in 1 file changed: 6 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/17364.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17364/head:pull/17364 PR: https://git.openjdk.org/jdk/pull/17364
Re: RFR: 8264102: JTable Keyboards Navigation differs with Test Instructions. [v6]
On Wed, 10 Jan 2024 05:25:42 GMT, Tejesh R wrote: >> Updated the actions for "ctrl shift DOWN" to >> "`selectLastRowExtendSelection`" and "ctrl shift UP", >> "`selectFirstRowExtendSelection`" in Basic and other Look and feel. Tested >> in CI and no regressions found. >> Test to be used - >> [javax/swing/JTable/KeyBoardNavigation/KeyBoardNavigation.java](https://github.com/openjdk/jdk/blob/master/test/jdk/javax/swing/JTable/KeyBoardNavigation/KeyBoardNavigation.java) > > Tejesh R has updated the pull request incrementally with one additional > commit since the last revision: > > Updated review comments LGTM. - Marked as reviewed by abhiscxk (Committer). PR Review: https://git.openjdk.org/jdk/pull/17236#pullrequestreview-1814651169
Re: RFR: JDK-8312518 : [macos13] setFullScreenWindow() shows black screen on macOS 13 & above
On Thu, 11 Jan 2024 01:01:46 GMT, Harshitha Onkar wrote: >It works the same in both cases - the shield window becomes the topmost window >and a black screen is seen. That probably is not right, but we can fix that separately. - PR Comment: https://git.openjdk.org/jdk/pull/17358#issuecomment-1886227441
Re: RFR: JDK-8312518 : [macos13] setFullScreenWindow() shows black screen on macOS 13 & above
On Wed, 10 Jan 2024 21:35:57 GMT, Harshitha Onkar wrote:
> A black screen is seen on newer versions of macOS (13.3 & above) when a
> window is set to full-screen using `setFullScreenWindow()`. The root cause
> was narrowed down to the shield level of the full-screen window vs the shield
> level of the captured display.
>
> Following solutions were explored -
>
> 1. Setting `kCGMaximumWindowLevelKey` as the shield level for the full screen
> window. But setting the fullscreen window to maximum available window level
> might cause z-order issues when other popup/screen savers are involved.
>
> int shieldLevel = CGWindowLevelForKey(kCGMaximumWindowLevelKey);
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: shieldLevel];
>
> 2. Raise the window's level slightly (shieldLevel + 1) above the system
> shield window.
>
> int shieldLevel = CGShieldingWindowLevel();
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: (shieldLevel + 1)];
>
> 3. Keeping the shielding level as-is and bringing the window to the
> foreground after display is captured. The 3rd approach **(also the one Apple
> recommends)** ensures that the full screen window has focus as well as being
> visible and also maintains the correct z-order. This solution works as
> expected on older (< 13.3) and newer versions (13.3 & above) of macOS.
>
> if (CGDisplayCapture(aID) == kCGErrorSuccess) {
> ...
> ...
> int shieldLevel = CGShieldingWindowLevel();
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: shieldLevel];
> [nsWindow makeKeyAndOrderFront: nil];
> }
Marked as reviewed by serb (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/17358#pullrequestreview-1814647094
Withdrawn: 8264728: When use chinese IME, the candidate box isn't moved with caret of JTextArea
On Thu, 16 Mar 2023 06:06:50 GMT, 柳鲲鹏 wrote: > Candidat box can moving with caret on windows version. Someone must wrote > codes for linux(ubuntu), but it doesn't work, so he didn't commit the codes. > Why it doesn't work, is the key problem. > > 1, I wrote a example for linux: > https://github.com/quantum6/X11InputMethod > > I tried all parameters to test and as my research: > If you use XIMPreeditCallbacks to initiate, the box can't be moved with caret. > If you use XIMPreeditNothing, it works. > All examples use XIMPreeditCallbacks to initiate input method and candidate > box can't moving. So I understand why he didn't commit the codes. > > 2, I traced the route of transfering caret coordites on windows version, then > add codes for linux. > 3, Taishan Office(like Microsoft Office Word) is running on jdk, we tested > for a long time, it works OK. > 4, I am not sure for AIX( no environment). > > > JDK-8264728 : When use chinese IME, the candidate box isn't moved with caret > of JTextArea > Type: Bug > Component: client-libs > Sub-Component: java.awt:i18n > Affected Version: 8,9,15,16 > Priority: P3 > Status: Open > Resolution: Unresolved > OS: linux > CPU: x86_64 This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/13055
Withdrawn: JDK-8313764: Offer JVM HS functionality to shared lib load operations done by the JDK codebase
On Mon, 14 Aug 2023 07:48:00 GMT, Matthias Baesken wrote: > Currently there is a number of functionality that would be interesting to > have for shared lib load operations in the JDK C code. > Some examples : > Events::log_dll_message for hs-err files reporting > JFR event NativeLibraryLoad > There is the need to update the shared lib Cache on AIX ( see > LoadedLibraries::reload() , see also > https://bugs.openjdk.org/browse/JDK-8314152 ), > this is currently not fully in sync with libs loaded form jdk c-libs and > sometimes reports outdated information > > Offer an interface (e.g. jvm.cpp) to support this. This pull request has been closed without being integrated. - PR: https://git.openjdk.org/jdk/pull/15264
Re: RFR: JDK-8312518 : [macos13] setFullScreenWindow() shows black screen on macOS 13 & above
On Wed, 10 Jan 2024 23:45:57 GMT, Sergey Bylokhov wrote:
>> A black screen is seen on newer versions of macOS (13.3 & above) when a
>> window is set to full-screen using `setFullScreenWindow()`. The root cause
>> was narrowed down to the shield level of the full-screen window vs the
>> shield level of the captured display.
>>
>> Following solutions were explored -
>>
>> 1. Setting `kCGMaximumWindowLevelKey` as the shield level for the full
>> screen window. But setting the fullscreen window to maximum available window
>> level might cause z-order issues when other popup/screen savers are involved.
>>
>> int shieldLevel = CGWindowLevelForKey(kCGMaximumWindowLevelKey);
>> window.preFullScreenLevel = [nsWindow level];
>> [nsWindow setLevel: shieldLevel];
>>
>> 2. Raise the window's level slightly (shieldLevel + 1) above the system
>> shield window.
>>
>> int shieldLevel = CGShieldingWindowLevel();
>> window.preFullScreenLevel = [nsWindow level];
>> [nsWindow setLevel: (shieldLevel + 1)];
>>
>> 3. Keeping the shielding level as-is and bringing the window to the
>> foreground after display is captured. The 3rd approach **(also the one Apple
>> recommends)** ensures that the full screen window has focus as well as being
>> visible and also maintains the correct z-order. This solution works as
>> expected on older (< 13.3) and newer versions (13.3 & above) of macOS.
>>
>> if (CGDisplayCapture(aID) == kCGErrorSuccess) {
>> ...
>> ...
>> int shieldLevel = CGShieldingWindowLevel();
>> window.preFullScreenLevel = [nsWindow level];
>> [nsWindow setLevel: shieldLevel];
>> [nsWindow makeKeyAndOrderFront: nil];
>> }
>
> How it will work now(and worked before) if the user will call toBack() on the
> such fullscreen window?
@mrserb
> How it will work now(and worked before) if the user will call toBack() on the
> such fullscreen window?
Tested both cases
- current fix on macOS 13.3 & above + fullScreenWindow.toBack()
- original code on older macOS version (11.5) + fullScreenWindow.toBack()
It works the same in both cases - the shield window becomes the topmost window
and a black screen is seen.
-
PR Comment: https://git.openjdk.org/jdk/pull/17358#issuecomment-1886019314
RFR: 8323554: The typos in Javadoc: "@return if "
The docs for the boolean methods are updated from this style:
@return if the event has been consumed
to this one
@return {@code true} if the event has been consumed, otherwise
{@code false}
plus small cleanups here and there.
-
Commit messages:
- one more
- one more typo
- 8323554: The typos in Javadoc: "@return if "
Changes: https://git.openjdk.org/jdk/pull/17357/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=17357=00
Issue: https://bugs.openjdk.org/browse/JDK-8323554
Stats: 123 lines in 14 files changed: 23 ins; 16 del; 84 mod
Patch: https://git.openjdk.org/jdk/pull/17357.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17357/head:pull/17357
PR: https://git.openjdk.org/jdk/pull/17357
Re: RFR: JDK-8312518 : [macos13] setFullScreenWindow() shows black screen on macOS 13 & above
On Wed, 10 Jan 2024 21:35:57 GMT, Harshitha Onkar wrote:
> A black screen is seen on newer versions of macOS (13.3 & above) when a
> window is set to full-screen using `setFullScreenWindow()`. The root cause
> was narrowed down to the shield level of the full-screen window vs the shield
> level of the captured display.
>
> Following solutions were explored -
>
> 1. Setting `kCGMaximumWindowLevelKey` as the shield level for the full screen
> window. But setting the fullscreen window to maximum available window level
> might cause z-order issues when other popup/screen savers are involved.
>
> int shieldLevel = CGWindowLevelForKey(kCGMaximumWindowLevelKey);
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: shieldLevel];
>
> 2. Raise the window's level slightly (shieldLevel + 1) above the system
> shield window.
>
> int shieldLevel = CGShieldingWindowLevel();
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: (shieldLevel + 1)];
>
> 3. Keeping the shielding level as-is and bringing the window to the
> foreground after display is captured. The 3rd approach **(also the one Apple
> recommends)** ensures that the full screen window has focus as well as being
> visible and also maintains the correct z-order. This solution works as
> expected on older (< 13.3) and newer versions (13.3 & above) of macOS.
>
> if (CGDisplayCapture(aID) == kCGErrorSuccess) {
> ...
> ...
> int shieldLevel = CGShieldingWindowLevel();
> window.preFullScreenLevel = [nsWindow level];
> [nsWindow setLevel: shieldLevel];
> [nsWindow makeKeyAndOrderFront: nil];
> }
How it will work now(and worked before) if the user will call toBack() on the
such fullscreen window?
-
PR Comment: https://git.openjdk.org/jdk/pull/17358#issuecomment-1885934211
RFR: JDK-8312518 : [macos13] setFullScreenWindow() shows black screen on macOS 13 & above
A black screen is seen on newer versions of macOS (13.3 & above) when a window
is set to full-screen using `setFullScreenWindow()`. The root cause was
narrowed down to the shield level of the full-screen window vs the shield level
of the captured display.
Following solutions were explored -
1. Setting `kCGMaximumWindowLevelKey` as the shield level for the full screen
window. But setting the fullscreen window to maximum available window level
might cause z-order issues when other popup/screen savers are involved.
int shieldLevel = CGWindowLevelForKey(kCGMaximumWindowLevelKey);
window.preFullScreenLevel = [nsWindow level];
[nsWindow setLevel: shieldLevel];
2. Raise the windows level slightly (shieldLevel + 1) above the system shield
window.
int shieldLevel = CGShieldingWindowLevel();
window.preFullScreenLevel = [nsWindow level];
[nsWindow setLevel: (shieldLevel + 1)];
3. Keeping the shielding level as-is and bringing the window to the foreground
after display is captured. The 3rd approach **(also the one Apple recommends)**
ensures that the full screen window has focus as well as being visible and also
maintains the correct z-order. This solution works as expected on older (<
13.3) and newer versions (13.3 & above) of macOS.
if (CGDisplayCapture(aID) == kCGErrorSuccess) {
...
...
int shieldLevel = CGShieldingWindowLevel();
window.preFullScreenLevel = [nsWindow level];
[nsWindow setLevel: shieldLevel];
[nsWindow makeKeyAndOrderFront: nil];
}
-
Commit messages:
- removed extra line
- test changes
- summary updated
- test changes
- initial changes
Changes: https://git.openjdk.org/jdk/pull/17358/files
Webrev: https://webrevs.openjdk.org/?repo=jdk=17358=00
Issue: https://bugs.openjdk.org/browse/JDK-8312518
Stats: 91 lines in 2 files changed: 90 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/17358.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17358/head:pull/17358
PR: https://git.openjdk.org/jdk/pull/17358
Re: RFR: 8316931: [macos14] Test "java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.html" throws and exception on Mac OS 14(x64, aarch64)
On Tue, 9 Jan 2024 21:06:50 GMT, Alisen Chung wrote: > SunToolkit.java is trying to post an event on the TrayIcon appContext, but > the TrayIcon was already removed by the test, causing an error. The fix is to > make SunToolkit skip posting the event if appContext is null. The test is > also updated to remove applet usage and use PassFailJFrame instead. Updated test, mach5 client tests are green - PR Comment: https://git.openjdk.org/jdk/pull/17329#issuecomment-1885731191
Re: RFR: 8316931: [macos14] Test "java/awt/TrayIcon/ShowAfterDisposeTest/ShowAfterDisposeTest.html" throws and exception on Mac OS 14(x64, aarch64) [v2]
> SunToolkit.java is trying to post an event on the TrayIcon appContext, but > the TrayIcon was already removed by the test, causing an error. The fix is to > make SunToolkit skip posting the event if appContext is null. The test is > also updated to remove applet usage and use PassFailJFrame instead. Alisen Chung has updated the pull request incrementally with two additional commits since the last revision: - spacing - updated test title, copyright year, removed redundant check - Changes: - all: https://git.openjdk.org/jdk/pull/17329/files - new: https://git.openjdk.org/jdk/pull/17329/files/171a1554..53629e25 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17329=01 - incr: https://webrevs.openjdk.org/?repo=jdk=17329=00-01 Stats: 8 lines in 1 file changed: 0 ins; 4 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/17329.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17329/head:pull/17329 PR: https://git.openjdk.org/jdk/pull/17329
Re: RFR: JDK-8323330: [BACKOUT] JDK-8276809: java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on Windows
On Wed, 10 Jan 2024 09:18:53 GMT, Matthias Baesken wrote: > There have been concerns raised about > [JDK-8276809](https://bugs.openjdk.org/browse/JDK-8276809) , so bring back > the old behavior. okay - Marked as reviewed by stuefe (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17341#pullrequestreview-1814102655
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix
On Thu, 30 Nov 2023 19:20:50 GMT, Sergey Bylokhov wrote: >> Hi Reviewers, >> There was a typo for color conversion instead of dstColorSpace function >> srcColorSpace was used. Please review and let me know your suggestions if >> any. >> >> Renjith. > > Can this patch be covered by the new test? >I'm not sure… We want the filter to take another path, there could be a list >of filters applied, if I understand @mrserb correctly. Sergey could be able to >provide a more detailed guidance. The current test validates two code paths: - wrapper>icc_color_space->wrapper - icc_color_space->icc_color_space->icc_color_space The color space in the middle is always icc_color_space: https://github.com/openjdk/jdk/pull/16895/files#diff-70b19b2642d6d3f44904de8b6eb2993e1c97320e3476898c4372db364c4288b7R130 If it we cover the wrapper for the middle as well we will cover this code path: https://github.com/openjdk/jdk/pull/16895/files#diff-e3d6eea060882cab00827c00e1a83b0e0a5b2a31fa9a9aa2122841bbd57c4a6dL853 - PR Comment: https://git.openjdk.org/jdk/pull/16895#issuecomment-1885608696
Re: RFR: JDK-8323330: [BACKOUT] JDK-8276809: java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on Windows
On Wed, 10 Jan 2024 09:18:53 GMT, Matthias Baesken wrote: > There have been concerns raised about > [JDK-8276809](https://bugs.openjdk.org/browse/JDK-8276809) , so bring back > the old behavior. Marked as reviewed by prr (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17341#pullrequestreview-1814046509
Re: RFR: 8320673 : PageFormat/CustomPaper.java has no Pass/Fail buttons; multiple instructions
On Wed, 27 Dec 2023 04:20:42 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > > I have updated the test case. Now test has pass/fail option also test will > execute two time for covering different cases which are part of the test. > > Please review and let me know your suggestions. > > Regards, > Renjith. Marked as reviewed by aivanov (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17194#pullrequestreview-1813963747
Re: RFR: 8316497 : ColorConvertOp - typo for non-ICC conversions needs one-line fix [v13]
On Tue, 9 Jan 2024 11:11:38 GMT, Renjith Kannath Pariyangad wrote: >> Hi Reviewers, >> There was a typo for color conversion instead of dstColorSpace function >> srcColorSpace was used. Please review and let me know your suggestions if >> any. >> >> Renjith. > > Renjith Kannath Pariyangad has updated the pull request incrementally with > one additional commit since the last revision: > > Fixed other two typos I merged master and ran the client tests—the tests are green. - PR Comment: https://git.openjdk.org/jdk/pull/16895#issuecomment-1885305466
Re: RFR: JDK-8320405: [Windows Server 2016] java/awt/image/MultiResolutionImage/MultiResolutionImageObserverTest.java shows issues in awt_Win32GraphicsDevice.cpp
On Thu, 4 Jan 2024 14:40:59 GMT, Matthias Baesken wrote: > > It should be extended to include the possible failure in the preceding call > > to ::GetDIBits. > > I handle now also a failure of the first GetDIBits call; and also added > braces at the if's mentioned. Sorry for my delayed reply. It's not what I expected. What I meant is that we shouldn't even call `::CreateCompatibleBitmap` if `hBMDC == NULL`. Then we shouldn't call `::GetDIBits` if `hBM == NULL`. Thus in this case, the _fallback_ code path should be taken. Perhaps, the code could be restructured… Yet I don't fully understand what's done here. - PR Comment: https://git.openjdk.org/jdk/pull/17197#issuecomment-1885284337
Re: RFR: JDK-8276809: java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on Windows
On Tue, 9 Jan 2024 06:35:12 GMT, Philip Race wrote: >reason for the exception which has NEVER been seen in our testing here and >once understood I checked the new output of the jtr with this change (where ExceptionDescribe is called) but nothing is reported unfortunately. So I really wonder - is there a exception present that we could try to understand ? >rather than suppressing it needs an actual fix and yet here it is again in the >form previously rejected. Without getting the exception (if there is one?) it is hard to propose a fix; it is not even totally clear if there is need for a fix. - PR Comment: https://git.openjdk.org/jdk/pull/17224#issuecomment-1885031083
Re: RFR: JDK-8323330: [BACKOUT] JDK-8276809: java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on Windows
On Wed, 10 Jan 2024 12:01:47 GMT, Julian Waters wrote: > Not a review, just a quick tip: Such changes are by convention titled > [BACKOUT] > > In this case that would be JDK-8323330: [BACKOUT] JDK-8276809: > java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on > Windows Thanks, I changed the title. - PR Comment: https://git.openjdk.org/jdk/pull/17341#issuecomment-1885020738
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v4]
On Wed, 10 Jan 2024 12:20:28 GMT, Abhishek Kumar wrote: >> src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line >> 82: >> >>> 80: private Icon expandedIconWrapper = new >>> IconWrapper(IconType.EXPANDED); >>> 81: >>> 82: private Icon collapsedIconWrapper = new >>> IconWrapper(IconType.COLLAPSED); >> >> I guess you can pass in `collapsedIcon` or `expandedIcon` to `IconWrapper >> `class which can save it in `icon` field and >> Then IconWrapper class can just call `SynthGraphicsUtils.paintIcon(icon, >> context, g, x, y, w, h);` > > I tried with this approach first but images didn't render at all. I guess those might be null at class initialisation stage so it didn't render..you can try expandedIconWrapper = new IconWrapper(expandedIcon); collapsedIconWrappr - new IconWrapper(collapsedICon); in updateStyle() where it sets setExpandedIcon(style.getIcon(context, "Tree.expandedIcon")); setCollapsedIcon(style.getIcon(context, "Tree.collapsedIcon")); - PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1447325540
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v4]
On Wed, 10 Jan 2024 11:26:17 GMT, Prasanta Sadhukhan wrote: >> Abhishek Kumar has updated the pull request incrementally with one >> additional commit since the last revision: >> >> ExpandedIconWrapper and CollapsedIconWrapper class merged together > > src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line > 82: > >> 80: private Icon expandedIconWrapper = new >> IconWrapper(IconType.EXPANDED); >> 81: >> 82: private Icon collapsedIconWrapper = new >> IconWrapper(IconType.COLLAPSED); > > I guess you can pass in `collapsedIcon` or `expandedIcon` to `IconWrapper > `class which can save it in `icon` field and > Then IconWrapper class can just call `SynthGraphicsUtils.paintIcon(icon, > context, g, x, y, w, h);` I tried with this approach first but images didn't render at all. - PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1447313431
Re: RFR: JDK-8323330: undo JDK-8276809 and bring back JNI warning
On Wed, 10 Jan 2024 09:18:53 GMT, Matthias Baesken wrote: > There have been concerns raised about > [JDK-8276809](https://bugs.openjdk.org/browse/JDK-8276809) , so bring back > the old behavior. Not a review, just a quick tip: Such changes are by convention titled [BACKOUT] In this case that would be JDK-8323330: [BACKOUT] JDK-8276809: java/awt/font/JNICheck/FreeTypeScalerJNICheck.java shows JNI warning on Windows - PR Comment: https://git.openjdk.org/jdk/pull/17341#issuecomment-1884718987
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v4]
On Wed, 10 Jan 2024 10:52:44 GMT, Abhishek Kumar wrote: >> The collapsed icon for JTree is not painted using `Icon.paintIcon(Component >> c, Graphics g, int x, int y)` in GTK LAF. The collapsed icon is returned >> from BasicTreeUI class which doesn't contain any icon image whereas the >> expanded icon is returned from SynthTreeUI class and expanded icon is >> rendered correctly. >> The proposed fix is to return collapsed icon as an object of collapsed icon >> wrapper which implements synthIcon and is similar to the expanded icon >> implementation. >> >> Test mentioned in JBS is an applet based which is converted to main based >> now and extended for all installed LAFs on the system. >> >> No regression caused with the fix, link is attached in JBS . > > Abhishek Kumar has updated the pull request incrementally with one additional > commit since the last revision: > > ExpandedIconWrapper and CollapsedIconWrapper class merged together src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line 82: > 80: private Icon expandedIconWrapper = new IconWrapper(IconType.EXPANDED); > 81: > 82: private Icon collapsedIconWrapper = new > IconWrapper(IconType.COLLAPSED); I guess you can pass in `collapsedIcon` or `expandedIcon` to `IconWrapper `class which can save it in `icon` field and Then IconWrapper class can just call `SynthGraphicsUtils.paintIcon(icon, context, g, x, y, w, h);` - PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1447249502
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v3]
On Wed, 10 Jan 2024 08:55:19 GMT, Prasanta Sadhukhan wrote: >> Should I merge the two classes into one class like instead of separate >> `ExpandedIconWrapper` and `CollapsedIconWrapper`, only one `IconWrapper` >> class? > > guess you could since they are all private.. Merged `ExpandedIconWrapper` and `CollapsedIconWrapper` class to `IconWrapper ` class. - PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1447202641
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v4]
> The collapsed icon for JTree is not painted using `Icon.paintIcon(Component > c, Graphics g, int x, int y)` in GTK LAF. The collapsed icon is returned from > BasicTreeUI class which doesn't contain any icon image whereas the expanded > icon is returned from SynthTreeUI class and expanded icon is rendered > correctly. > The proposed fix is to return collapsed icon as an object of collapsed icon > wrapper which implements synthIcon and is similar to the expanded icon > implementation. > > Test mentioned in JBS is an applet based which is converted to main based now > and extended for all installed LAFs on the system. > > No regression caused with the fix, link is attached in JBS . Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision: ExpandedIconWrapper and CollapsedIconWrapper class merged together - Changes: - all: https://git.openjdk.org/jdk/pull/17294/files - new: https://git.openjdk.org/jdk/pull/17294/files/ed5caa98..2fa05f9f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17294=03 - incr: https://webrevs.openjdk.org/?repo=jdk=17294=02-03 Stats: 76 lines in 1 file changed: 32 ins; 33 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/17294.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17294/head:pull/17294 PR: https://git.openjdk.org/jdk/pull/17294
RFR: JDK-8323330: undo JDK-8276809 and bring back JNI warning
There have been concerns raised about [JDK-8276809](https://bugs.openjdk.org/browse/JDK-8276809) , so bring back the old behavior. - Commit messages: - JDK-8323330 Changes: https://git.openjdk.org/jdk/pull/17341/files Webrev: https://webrevs.openjdk.org/?repo=jdk=17341=00 Issue: https://bugs.openjdk.org/browse/JDK-8323330 Stats: 8 lines in 1 file changed: 0 ins; 6 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/17341.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17341/head:pull/17341 PR: https://git.openjdk.org/jdk/pull/17341
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v3]
On Wed, 10 Jan 2024 08:25:52 GMT, Abhishek Kumar wrote:
>> src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line
>> 837:
>>
>>> 835: }
>>> 836: else {
>>> 837: SynthGraphicsUtils.paintIcon(collapsedIcon, context,
>>> g, x, y, w, h);
>>
>> Guess it's a copy of `ExpandedIconWrapper `class, so `CollapsedIconWrapper
>> `and `ExpandedIconWrapper` can be optimised to use common method passing in
>> the "icon" argument..
>
> Should I merge the two classes into one class like instead of separate
> `ExpandedIconWrapper` and `CollapsedIconWrapper`, only one `IconWrapper`
> class?
guess you could since they are all private..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1447064941
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v3]
On Wed, 10 Jan 2024 05:56:37 GMT, Prasanta Sadhukhan
wrote:
>> Abhishek Kumar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Remove whitespace error
>
> src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line
> 837:
>
>> 835: }
>> 836: else {
>> 837: SynthGraphicsUtils.paintIcon(collapsedIcon, context, g,
>> x, y, w, h);
>
> Guess it's a copy of `ExpandedIconWrapper `class, so `CollapsedIconWrapper
> `and `ExpandedIconWrapper` can be optimised to use common method passing in
> the "icon" argument..
Should I merge the two classes into one class like instead of separate
`ExpandedIconWrapper` and `CollapsedIconWrapper`, only one `IconWrapper` class?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1447033548
