Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
On 10/26/2017 12:45 PM, Sergey Bylokhov wrote: On 26/10/2017 08:26, Semyon Sadetsky wrote: The existing applications will use the new functionality which behaves like the old ones. The appearance of the message itself will differ but after the fix it will look like the native message, even before the fix the appearance was different on different platforms. I'm not sure that it will behave as before since that was a swing component which uses Swing L, so at least the appearance is changed by the fix > Yet another thing is localization. Before the fix the message was shown in java locale which may differ from the native locale, now it is always follows the native locale and there is no way to change it from java. The things above are purpose of the fix - to look identical to a native applications(appearance, location and behavior). The old implementation of this class via Swing was just a stub which was implemented to fill the gap in API between jdk6 and 7. Since this is awt component it should be implemented via the native API. This may cause unexpected changes in the existing applications, so I suppose it is reasonable to introduce a compatibility property that allows switching to the previous behavior. At a minimum, this should be mentioned in compatibility section of the release notes. I'll create a release notes subtask, do you have some other comments? Thanks. Looks fine to me. --Semyon
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
On 26/10/2017 08:26, Semyon Sadetsky wrote: The existing applications will use the new functionality which behaves like the old ones. The appearance of the message itself will differ but after the fix it will look like the native message, even before the fix the appearance was different on different platforms. I'm not sure that it will behave as before since that was a swing component which uses Swing L, so at least the appearance is changed by the fix > Yet another thing is localization. Before the fix the message was shown in java locale which may differ from the native locale, now it is always follows the native locale and there is no way to change it from java. The things above are purpose of the fix - to look identical to a native applications(appearance, location and behavior). The old implementation of this class via Swing was just a stub which was implemented to fill the gap in API between jdk6 and 7. Since this is awt component it should be implemented via the native API. This may cause unexpected changes in the existing applications, so I suppose it is reasonable to introduce a compatibility property that allows switching to the previous behavior. At a minimum, this should be mentioned in compatibility section of the release notes. I'll create a release notes subtask, do you have some other comments? -- Best regards, Sergey.
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
On 10/25/2017 06:49 PM, Sergey Bylokhov wrote: On 16/10/2017 12:14, semyon.sadet...@oracle.com wrote: But what to do with compatibility issues? Do you provide any options to run existing applications? The existing applications will use the new functionality which behaves like the old ones. The appearance of the message itself will differ but after the fix it will look like the native message, even before the fix the appearance was different on different platforms. I'm not sure that it will behave as before since that was a swing component which uses Swing L, so at least the appearance is changed by the fix. Yet another thing is localization. Before the fix the message was shown in java locale which may differ from the native locale, now it is always follows the native locale and there is no way to change it from java. This may cause unexpected changes in the existing applications, so I suppose it is reasonable to introduce a compatibility property that allows switching to the previous behavior. At a minimum, this should be mentioned in compatibility section of the release notes. --Semyon
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
On 16/10/2017 12:14, semyon.sadet...@oracle.com wrote: But what to do with compatibility issues? Do you provide any options to run existing applications? The existing applications will use the new functionality which behaves like the old ones. The appearance of the message itself will differ but after the fix it will look like the native message, even before the fix the appearance was different on different platforms. -- Best regards, Sergey.
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
On 10/16/17, 2:29 PM, Sergey Bylokhov wrote: On 16/10/2017 12:30, Phil Race wrote: - Why do we scale the icon by 0.75 ? I could be missing something but it appears that this will be based off the same image whether it is retina or not and I wonder if we really should be scaling it down on retina ? 0.75: This code makes the application icon smaller on the notification, things related to style/design. The same image is used from the LookAndFeel whether the screen is retina or not. To use HiDPI image it is necessary: - Read all representation from the native NSImage and store them in MultiResolutionImage. - Store this image in the L property. - In CTrayIcon read the MRI from the l and convert it back to the NSImage. - Pass the nsimage to the native notification, the OS will select correct representation. I can start to work on this after the current fix. OK .. you will be filing a new bug I take it. - Are we losing anything by no longer knowing when the user has dismissed the notification ? We can generate an action when the user clicks on the notification, but I preserved the old behavior, the click on the notification just close it as before. Ok - I suppose a test case for this is hard ? TrayIcon.displayMessage() is a platform dependent method, there are nothing to test except that we could show message/notification somewhere near tray area. We already have some tests which use displayMessage(). OK - Can perhaps attach an "after the fix" screen shot to the bug report ? done. All seems fine then. -phil. On 09/27/2017 10:07 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187639 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187639/webrev.01 Since macOS 10.9(or even early) the main menubar is shown on all screens(not only on the main screen), which means that on both screens the trayIcons are visible. Our code is not ready for this situation, because we create a custom notification window and tries to place it near the trayicon on the main screen. Instead of updating the logic of showing the window, I migrated the code to the standard notification mechanism which is used in macOS. Examples: The old message: http://cr.openjdk.java.net/~serb/8187639/images/Old.png The new(java -jar): http://cr.openjdk.java.net/~serb/8187639/images/Command%20line.png The new(bundles application): http://cr.openjdk.java.net/~serb/8187639/images/Bundled%20applicateion.png
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
On 16/10/2017 12:30, Phil Race wrote: - Why do we scale the icon by 0.75 ? I could be missing something but it appears that this will be based off the same image whether it is retina or not and I wonder if we really should be scaling it down on retina ? 0.75: This code makes the application icon smaller on the notification, things related to style/design. The same image is used from the LookAndFeel whether the screen is retina or not. To use HiDPI image it is necessary: - Read all representation from the native NSImage and store them in MultiResolutionImage. - Store this image in the L property. - In CTrayIcon read the MRI from the l and convert it back to the NSImage. - Pass the nsimage to the native notification, the OS will select correct representation. I can start to work on this after the current fix. - Are we losing anything by no longer knowing when the user has dismissed the notification ? We can generate an action when the user clicks on the notification, but I preserved the old behavior, the click on the notification just close it as before. - I suppose a test case for this is hard ? TrayIcon.displayMessage() is a platform dependent method, there are nothing to test except that we could show message/notification somewhere near tray area. We already have some tests which use displayMessage(). - Can perhaps attach an "after the fix" screen shot to the bug report ? done. On 09/27/2017 10:07 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187639 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187639/webrev.01 Since macOS 10.9(or even early) the main menubar is shown on all screens(not only on the main screen), which means that on both screens the trayIcons are visible. Our code is not ready for this situation, because we create a custom notification window and tries to place it near the trayicon on the main screen. Instead of updating the logic of showing the window, I migrated the code to the standard notification mechanism which is used in macOS. Examples: The old message: http://cr.openjdk.java.net/~serb/8187639/images/Old.png The new(java -jar): http://cr.openjdk.java.net/~serb/8187639/images/Command%20line.png The new(bundles application): http://cr.openjdk.java.net/~serb/8187639/images/Bundled%20applicateion.png -- Best regards, Sergey.
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
- Why do we scale the icon by 0.75 ? I could be missing something but it appears that this will be based off the same image whether it is retina or not and I wonder if we really should be scaling it down on retina ? - Are we losing anything by no longer knowing when the user has dismissed the notification ? - I suppose a test case for this is hard ? - Can perhaps attach an "after the fix" screen shot to the bug report ? -phil. On 09/27/2017 10:07 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187639 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187639/webrev.01 Since macOS 10.9(or even early) the main menubar is shown on all screens(not only on the main screen), which means that on both screens the trayIcons are visible. Our code is not ready for this situation, because we create a custom notification window and tries to place it near the trayicon on the main screen. Instead of updating the logic of showing the window, I migrated the code to the standard notification mechanism which is used in macOS. Examples: The old message: http://cr.openjdk.java.net/~serb/8187639/images/Old.png The new(java -jar): http://cr.openjdk.java.net/~serb/8187639/images/Command%20line.png The new(bundles application): http://cr.openjdk.java.net/~serb/8187639/images/Bundled%20applicateion.png
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
But what to do with compatibility issues? Do you provide any options to run existing applications? On 10/12/17 1:51 PM, Sergey Bylokhov wrote: This is one of the task of awt to provide a native appearance, the FileDialog is one of the examples. The current fix provide the same functionality like the old code, it looks is native, it works in a multi-screen environment. I think that we should implement the same fix for linux/windows as well at some point. On 09/10/2017 09:19, Semyon Sadetsky wrote: Hi Sergey, The suggested replacing the Swing dialog with the native notification popup seems too radical change for the issue. It might be justified in case of the System LnF, but even in this case I'm not sure, because of possible compatibility problems. --Semyon On 09/27/2017 10:07 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187639 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187639/webrev.01 Since macOS 10.9(or even early) the main menubar is shown on all screens(not only on the main screen), which means that on both screens the trayIcons are visible. Our code is not ready for this situation, because we create a custom notification window and tries to place it near the trayicon on the main screen. Instead of updating the logic of showing the window, I migrated the code to the standard notification mechanism which is used in macOS. Examples: The old message: http://cr.openjdk.java.net/~serb/8187639/images/Old.png The new(java -jar): http://cr.openjdk.java.net/~serb/8187639/images/Command%20line.png The new(bundles application): http://cr.openjdk.java.net/~serb/8187639/images/Bundled%20applicateion.png
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
This is one of the task of awt to provide a native appearance, the FileDialog is one of the examples. The current fix provide the same functionality like the old code, it looks is native, it works in a multi-screen environment. I think that we should implement the same fix for linux/windows as well at some point. On 09/10/2017 09:19, Semyon Sadetsky wrote: Hi Sergey, The suggested replacing the Swing dialog with the native notification popup seems too radical change for the issue. It might be justified in case of the System LnF, but even in this case I'm not sure, because of possible compatibility problems. --Semyon On 09/27/2017 10:07 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187639 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187639/webrev.01 Since macOS 10.9(or even early) the main menubar is shown on all screens(not only on the main screen), which means that on both screens the trayIcons are visible. Our code is not ready for this situation, because we create a custom notification window and tries to place it near the trayicon on the main screen. Instead of updating the logic of showing the window, I migrated the code to the standard notification mechanism which is used in macOS. Examples: The old message: http://cr.openjdk.java.net/~serb/8187639/images/Old.png The new(java -jar): http://cr.openjdk.java.net/~serb/8187639/images/Command%20line.png The new(bundles application): http://cr.openjdk.java.net/~serb/8187639/images/Bundled%20applicateion.png -- Best regards, Sergey.
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
Hi Sergey, The suggested replacing the Swing dialog with the native notification popup seems too radical change for the issue. It might be justified in case of the System LnF, but even in this case I'm not sure, because of possible compatibility problems. --Semyon On 09/27/2017 10:07 AM, Sergey Bylokhov wrote: Hello, Please review the fix for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187639 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187639/webrev.01 Since macOS 10.9(or even early) the main menubar is shown on all screens(not only on the main screen), which means that on both screens the trayIcons are visible. Our code is not ready for this situation, because we create a custom notification window and tries to place it near the trayicon on the main screen. Instead of updating the logic of showing the window, I migrated the code to the standard notification mechanism which is used in macOS. Examples: The old message: http://cr.openjdk.java.net/~serb/8187639/images/Old.png The new(java -jar): http://cr.openjdk.java.net/~serb/8187639/images/Command%20line.png The new(bundles application): http://cr.openjdk.java.net/~serb/8187639/images/Bundled%20applicateion.png
Re: [10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
Looks fine Thanks, Alexander. On 27/09/2017 22:37, Sergey Bylokhov wrote: Hello, Please review the fix for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187639 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187639/webrev.01 Since macOS 10.9(or even early) the main menubar is shown on all screens(not only on the main screen), which means that on both screens the trayIcons are visible. Our code is not ready for this situation, because we create a custom notification window and tries to place it near the trayicon on the main screen. Instead of updating the logic of showing the window, I migrated the code to the standard notification mechanism which is used in macOS. Examples: The old message: http://cr.openjdk.java.net/~serb/8187639/images/Old.png The new(java -jar): http://cr.openjdk.java.net/~serb/8187639/images/Command%20line.png The new(bundles application): http://cr.openjdk.java.net/~serb/8187639/images/Bundled%20applicateion.png
[10] Review Request: 8187639 TrayIcon is not properly supported on macOS in multi-screen environment
Hello, Please review the fix for jdk10. Bug: https://bugs.openjdk.java.net/browse/JDK-8187639 Webrev can be found at: http://cr.openjdk.java.net/~serb/8187639/webrev.01 Since macOS 10.9(or even early) the main menubar is shown on all screens(not only on the main screen), which means that on both screens the trayIcons are visible. Our code is not ready for this situation, because we create a custom notification window and tries to place it near the trayicon on the main screen. Instead of updating the logic of showing the window, I migrated the code to the standard notification mechanism which is used in macOS. Examples: The old message: http://cr.openjdk.java.net/~serb/8187639/images/Old.png The new(java -jar): http://cr.openjdk.java.net/~serb/8187639/images/Command%20line.png The new(bundles application): http://cr.openjdk.java.net/~serb/8187639/images/Bundled%20applicateion.png -- Best regards, Sergey.
