On Thu, 15 Feb 2024 15:51:18 GMT, Prasanta Sadhukhan <[email protected]>
wrote:
>>> But here the question is about getTrayIconSize for "disabling system tray"
>>> [i.e. should we do
>> if (shouldDisableSystemTray) {
>> return;
>> }
>>> and as I understand we are disabling it even though it can be supported (ie
>>> isSupported returns true) before version <45, no?
>>
>> They are tied together:
>>
>> If we have Gnome Shell < 45:
>> -> SystemTray is not supported(`isSupported` returns `false`)
>> -> You can't get the instance, `SystemTray.getSystemTray()` throws
>> `UnsupportedOperationException`
>> -> you can't call `getTrayIconSize` (The only call to `getTrayIconSize` is
>> made from
>> [java.awt.SystemTray#getTrayIconSize](https://github.com/openjdk/jdk/blob/a0e5e16afbd19f6396f0af2cba954225a357eca8/src/java.desktop/share/classes/java/awt/SystemTray.java#L357))
>>
>> So I don't think that we should add this check to the `getTrayIconSize`.
>>
>>> Else I guess the right place to call shouldDisableSystemTray should be in
>>> isSupported and if shouldDisableSystemTray returns true, isSupported should
>>> also return false?
>>
>> `java.awt.SystemTray#isSupported`? It is already returns false in this case,
>> but by asking a peer.
>>
>> I don't think that we should touch the shared code for this, it would
>> unnecessarily increase the testing surface.
>>
>> Another good option is to return a null peer in
>> `sun.awt.X11.XToolkit#createSystemTray` if needed, but there isn't enough
>> time to test this thoroughly. We can revisit this in
>> [JDK-8325914](https://bugs.openjdk.org/browse/JDK-8325914)
>
>> If we have Gnome Shell < 45:
>> -> SystemTray is not supported(`isSupported` returns `false`)
>
> If isSupported returns false for version < 45, then why we need this code
> because as I understand, if systemTray is unsupported, then system tray is
> not there, so the need to check for any icon present in tray doesn't arise so
> there should not be need of calling `shouldDisableSystemTray` , right? I am
> bit confused..
All current `shouldDisableSystemTray` checks in `XSystemTrayPeer` are aimed at
preventing the `available` value from changing and leaving it as `false`,
because the `isSupported` is decided based on the `XSystemTrayPeer#isAvailable`
response.
We can replace the `ownerChanged` and `ownerDeath` checks with the check in
`isAvailable`, but not this time.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17860#discussion_r1491252011
