[icedtea-web] RFC: PR857: Don't set look and feel multiple times
Omair Majid
omajid at redhat.com
Mon Mar 3 09:42:55 PST 2014
* Jiri Vanek <jvanek at redhat.com> [2014-03-03 02:51]:
> On 03/01/2014 12:07 AM, Omair Majid wrote:
> >Hi,
> >
> >The attached patch is a fix for PR857. The idea is quite simple: make
> >sure UIManager.setLookAndFeel() is called at most once (and ideally
> >exactly once) before any UI is shown.
> >
> >There are a few code paths that can cause UIManager.setLookAndFell to be
> >invoked multiple times:
> >
> >- AboutDialog (only on entry from a class other than Boot)
> >- SecurityDialog
> >- PolicyEditor (only on entry from ControlPanel)
> >
> >CertificateViewer calls JNLPRuntime.initialize() which already calls
> >UIManager.setLookAndFeel. So I have removed the direct call from
> >CertificateViewer.
> >
> >Also, JNLPRuntime.initialize currently calls setLookAndFeel after
> >displaying a JavaConsole. This is alos fixed.
> >
> >Thoughts?
> >
>
> ouch, that an old one!
>
>
> However - the original one - http://icedtea.classpath.org/bugzilla/attachment.cgi?id=640&action=diff
> - (the confirmed one) is quite different, why so?
For a couple of reasons:
- The reporter only tested one code path (the main plugin/javaws code
path)
- I went through more code this time and noticed more subtle issues,
including cases where setting the look and feel is completely
redundant.
> Also " Don't set look and feel multiple times" do not sound proper
> to me, it is still set two times.
I don't follow. It is set exactly once in (a shared code path) in the
case of plugins/javaws: in JNLPRuntime.initialize().
> Can't there be only one place to do so?
No, unless you want to have exactly one entry point for icedtea-web that then selects what to run:
- javaws
- plugin
- about dialog
- controlpanel
- certificate viewer
- policy editor
> I would rather log the exception.
>
> I would ratehr log this exception in debug only (default) =?
> - OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
> + OutputController.getLogger().log(e);
Fixed.
Thanks,
Omair
--
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95 0056 F286 F14F 6648 4681
-------------- next part --------------
diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2014-03-03 Omair Majid <omajid at redhat.com>
+
+ PR857
+ * netx/net/sourceforge/jnlp/about/AboutDialog.java
+ (run): Do not set look and feel.
+ * netx/net/sourceforge/jnlp/runtime/Boot.java
+ (main) <about>: Set look and feel before displaying dialog.
+ * netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
+ (initialize): Set look and feel before any UI is created.
+ * netx/net/sourceforge/jnlp/security/SecurityDialog.java
+ (init): Do not set look and feel.
+ (setSystemLookAndFeel): Removed.
+ * netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
+ (createInstance): Do not set look and feel.
+ * netx/net/sourceforge/jnlp/security/viewer/CertificateViewer.java
+ (showCertificateViewer): Do not set look and feel.
+ (setSystemLookAndFeel): Removed.
+
2014-03-03 Omair Majid <omajid at redhat.com>
PR1676
diff --git a/netx/net/sourceforge/jnlp/about/AboutDialog.java b/netx/net/sourceforge/jnlp/about/AboutDialog.java
--- a/netx/net/sourceforge/jnlp/about/AboutDialog.java
+++ b/netx/net/sourceforge/jnlp/about/AboutDialog.java
@@ -175,11 +175,6 @@
@Override
public void run() {
- try {
- UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
- } catch (Exception e) {
- }
-
layoutWindow();
ScreenFinder.centerWindowsToCurrentScreen(frame);
frame.setVisible(true);
diff --git a/netx/net/sourceforge/jnlp/runtime/Boot.java b/netx/net/sourceforge/jnlp/runtime/Boot.java
--- a/netx/net/sourceforge/jnlp/runtime/Boot.java
+++ b/netx/net/sourceforge/jnlp/runtime/Boot.java
@@ -28,6 +28,8 @@
import java.util.List;
import java.util.Map;
+import javax.swing.UIManager;
+
import net.sourceforge.jnlp.LaunchException;
import net.sourceforge.jnlp.Launcher;
import net.sourceforge.jnlp.ParserSettings;
@@ -161,6 +163,11 @@
if (null != getOption("-headless")) {
JNLPRuntime.exit(0);
} else {
+ try {
+ UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
+ } catch (Exception e) {
+ OutputController.getLogger().log("Unable to set system look and feel");
+ }
OutputController.getLogger().printOutLn(R("BLaunchAbout"));
AboutDialog.display();
return;
diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java b/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
--- a/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java
@@ -195,6 +195,12 @@
public static void initialize(boolean isApplication) throws IllegalStateException {
checkInitialized();
+ try {
+ UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
+ } catch (Exception e) {
+ OutputController.getLogger().log("Unable to set system look and feel");
+ }
+
if (JavaConsole.canShowOnStartup(isApplication)) {
JavaConsole.getConsole().showConsoleLater();
}
@@ -236,12 +242,6 @@
policy = new JNLPPolicy();
security = new JNLPSecurityManager(); // side effect: create JWindow
- try {
- UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
- } catch (Exception e) {
- OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e);
- }
-
doMainAppContextHacks();
if (securityEnabled) {
diff --git a/netx/net/sourceforge/jnlp/security/SecurityDialog.java b/netx/net/sourceforge/jnlp/security/SecurityDialog.java
--- a/netx/net/sourceforge/jnlp/security/SecurityDialog.java
+++ b/netx/net/sourceforge/jnlp/security/SecurityDialog.java
@@ -216,8 +216,6 @@
}
private void initDialog() {
- setSystemLookAndFeel();
-
String dialogTitle = "";
if (dialogType == DialogType.CERT_WARNING) {
if (accessType == AccessType.VERIFIED)
@@ -346,17 +344,6 @@
super.dispose();
}
- /**
- * Updates the look and feel of the window to be the system look and feel
- */
- protected void setSystemLookAndFeel() {
- try {
- UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
- } catch (Exception e) {
- //don't worry if we can't.
- }
- }
-
private final List<ActionListener> listeners = new CopyOnWriteArrayList<ActionListener>();
/**
diff --git a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
--- a/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
+++ b/netx/net/sourceforge/jnlp/security/policyeditor/PolicyEditor.java
@@ -917,11 +917,6 @@
}
public static PolicyEditor createInstance(final String filepath) {
- try {
- UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
- } catch (final Exception e) {
- // not really important, so just ignore
- }
return new PolicyEditor(filepath);
}
diff --git a/netx/net/sourceforge/jnlp/security/viewer/CertificateViewer.java b/netx/net/sourceforge/jnlp/security/viewer/CertificateViewer.java
--- a/netx/net/sourceforge/jnlp/security/viewer/CertificateViewer.java
+++ b/netx/net/sourceforge/jnlp/security/viewer/CertificateViewer.java
@@ -100,7 +100,6 @@
public static void showCertificateViewer() throws Exception {
JNLPRuntime.initialize(true);
- setSystemLookAndFeel();
CertificateViewer cv = new CertificateViewer();
cv.setResizable(true);
@@ -109,14 +108,6 @@
cv.dispose();
}
- private static void setSystemLookAndFeel() {
- try {
- UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
- } catch (Exception e) {
- // don't worry if we can't.
- }
- }
-
public static void main(String[] args) throws Exception {
CertificateViewer.showCertificateViewer();
}
More information about the distro-pkg-dev
mailing list