<Swing Dev> RFR: 8247753: UIManager.getSytemLookAndFeelClassName() returns wrong value on Fedora 32

[Philip Race]

The status quo that if it isn't GTK, we use Metal seems like the right 
thing to stick with.
There'd be larger changes needed to support GTK if (for example) the 
right libraries
weren't installed, and who knows what UI defaults would be set to and if 
themes
would work properly and even then what's the point ?

All in all safe and simpler to use Metal.

-phil.

On 8/11/20, 7:47 AM, Mario Torre wrote:
> Hi Pankaj,
>
> Sorry for replying late to this!
>
> I think the patch is good, I'm still unsure if we shouldn't try to
> support the GTK laf on anything non KDE instead, but I believe this
> patch preserves compatibility with the current state of things.
>
> Cheers,
> Mario
>
> On Thu, Aug 6, 2020 at 12:03 PM Prasanta Sadhukhan
> <prasanta.sadhukhan at oracle.com>  wrote:
>> +1
>>
>> Regards
>> Prasanta
>> On 06-Aug-20 1:23 AM, Philip Race wrote:
>>
>> Approved.
>>
>> -phil.
>>
>> On 8/5/20, 11:22 AM, Pankaj Bansal wrote:
>>
>> Hello Phil,
>>
>> I have made the changes you suggested.
>>
>> webrev: http://cr.openjdk.java.net/~pbansal/8247753/webrev02/
>>
>> <<I am assuming that you have a reason for both toLowerCase() and contains() rather than equals() ?
>>
>> Yes, in some distros XDG_CURRENT_DESKTOP is set to some string containing "gnome" string, but not exactly "gnome". For example, on Ubuntu 18.04, it is "ubuntu:GNOME". I could have used "endsWith" method as of now, but for more inclusiveness, I have used contains, so we don't have to change again if some distro decides to set XDG_CURRENT_DESKTOP to something not covered by "endsWith".
>>
>> Regards,
>>
>> Pankaj
>>
>>
>>
>> On 05/08/20 9:35 PM, Philip Race wrote:
>>
>> I've read the bug comments and it looks like you've eventually come to the right answer.
>> I just would code it differently for a couple of reasons
>> 1) To make it easier some day to remove checking the old variable by just deleting a few lines
>> 2) To avoid repeating the string literal which I always think of as error-prone
>>
>> I am assuming that you have a reason for both toLowerCase() and contains() rather than equals() ?
>>
>> diff --git a/src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java b/src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java
>> --- a/src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java
>> +++ b/src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java
>> @@ -95,10 +95,19 @@
>>
>>       @Override
>>       public String getDesktop() {
>> +        String gnome = "gnome";
>>           String gsi = AccessController.doPrivileged(
>>                           (PrivilegedAction<String>) ()
>>                                   ->  System.getenv("GNOME_DESKTOP_SESSION_ID"));
>> -        return (gsi != null) ? "gnome" : null;
>> +        if (gsi != null) {
>> +            return gnome;
>> +        }
>> +        String desktop = AccessController.doPrivileged(
>> +             (PrivilegedAction<String>) ()
>> +                     ->  System.getenv("XDG_CURRENT_DESKTOP"));
>> +        return (desktop != null&&  desktop.toLowerCase().contains(gnome))
>> +               ? gnome : null;
>> +
>>       }
>>
>> -phil.
>>
>> On 8/4/20, 10:16 PM, Prasanta Sadhukhan wrote:
>>
>> Looks ok to me.
>>
>> One observation is, in testcase you can remove line71 linux check as we already did the check in l54.
>>
>> Regards
>> Prasanta
>> On 04-Aug-20 11:12 PM, Pankaj Bansal wrote:
>>
>> Hi All,
>>
>> Please review the following fix for jdk16.
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8247753
>> webrev: http://cr.openjdk.java.net/~pbansal/8247753/webrev00
>>
>> Bug: UIManager.getSytemLookAndFeelClassName() returns wrong value on Fedora 32. It is returning the MetalLookAndFeel classname instead of GTKLookAndFeel classname.
>>
>> Cause: Java uses a environment variable GNOME_DESKTOP_SESSION_ID to verify if the system is gnome (which is set by gnome) and then checks for GTKLookAndFeel support. If both conditions are satisfied, then GTKLookAndFeel classname is returned, else cross platform MetalLookAndFeel is selected.
>>
>> The GNOME_DESKTOP_SESSION_ID environment has been deprecated for a long time and the value is set to "this-is-deprecated" by gnome. In java, the actual value of variable is not being verified. As long as the value is not null (it has been set to something by gnome), things have been working fine though the value  set by gnome is "this-is-deprecated". Now, gnome has removed the variable completely and this is causing issues in Fedora 32. As the variable is not set at all, java is returning the cross platform MetalLookAndFeel classname for the SystemLookAndFeelClassName instead of GTKLookAndFeel.
>>
>> More information on this in JBS.  Right now the issue is seen only on Fedora 32, but this can very well come in future releases of RHEL, CentOS, Oracle Linux etc.
>>
>> Fix: We should check XDG_CURRENT_DESKTOP also along with GNOME_DESKTOP_SESSION_ID to verify gnome based linux. XDG_CURRENT_DESKTOP is set by gnome based platforms to some string containing "gnome" substring. So, this can be used to verify the gnome based desktop. For backward compatibility, we need to continue checking GNOME_DESKTOP_SESSION_ID as well because for some linux platforms, XDG_CURRENT_DESKTOP may be set to something else as they are not gnome based, but have been returning GTKL&F as GNOME_DESKTOP_SESSION_ID was set by them. so, we dont want to change anything for them.
>>
>> Other solution could have been to just make GTKL&F default on all linux, but that would result in GTKL&F being selected on some new platfoms like KDE based platforms where currently Metal L&F is selected.
>>
>> I have modified the test case test/jdk/javax/swing/LookAndFeel/SystemLookAndFeel/SystemLookAndFeelTest.java. The test fails on Fedora 32 without fix and passes with the fix. I have run the required mach5 tests and this fix is not breaking anything. Link in JBS.
>>
>>
>> Regards,
>>
>> Pankaj Bansal
>>
>>
>