<Swing Dev> RFR: 8247753: UIManager.getSytemLookAndFeelClassName() returns wrong value on Fedora 32
Mario Torre
neugens.limasoftware at gmail.com
Tue Aug 11 18:47:09 UTC 2020
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.race at 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.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
> >>
> >>
> >
>
--
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/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20200811/36fba5d3/attachment-0001.htm>
More information about the swing-dev
mailing list