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

[Philip Race]

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/20200805/f892afb1/attachment-0001.htm>