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

[Pankaj Bansal]

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