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

[Philip Race]

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/64a504f3/attachment-0001.htm>