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
