Yeah, And I would argue this is the role of the packager to ensure all the right things are there and integrated if they want to support a GTK laf in their specific environment, keeping Metal as default non Gnome in upstream makes sense.
Cheers, Mario On Tue 11. Aug 2020 at 16:59, Philip Race <philip.r...@oracle.com> wrote: > The status quo that if it isn't GTK, we use Metal seems like the right > thing to stick with. > There'd be larger changes needed to support GTK if (for example) the > right libraries > weren't installed, and who knows what UI defaults would be set to and if > themes > would work properly and even then what's the point ? > > All in all safe and simpler to use Metal. > > -phil. > > On 8/11/20, 7:47 AM, Mario Torre wrote: > > 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.sadhuk...@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 > >> > >> > > > -- pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF Fingerprint: BA39 9666 94EC 8B73 27FA FC7C 4086 63E3 80F2 40CF Java Champion - Blog: http://neugens.wordpress.com - Twitter: @neugens Proud GNU Classpath developer: http://www.classpath.org/ OpenJDK: http://openjdk.java.net/projects/caciocavallo/ Please, support open standards: http://endsoftpatents.org/
