+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
