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

Tue Aug 11 14:47:22 UTC 2020

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
>
>

-- 
Mario Torre
Associate Manager, Software Engineering
Red Hat GmbH <https://www.redhat.com>
9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898