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

Prasanta Sadhukhan prasanta.sadhukhan at oracle.com
Thu Aug 6 10:01:56 UTC 2020


+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/ 
>> <http://cr.openjdk.java.net/%7Epbansal/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 
>>>>> <http://cr.openjdk.java.net/%7Epbansal/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
>>>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20200806/7073db3d/attachment-0001.htm>


More information about the swing-dev mailing list