[icedtea-web] resurrection of (output)console?
Omair Majid
omajid at redhat.com
Mon Nov 4 14:20:07 PST 2013
Hi,
Comments in-line below.
* Jiri Vanek <jvanek at redhat.com> [2013-11-04 10:10]:
> On 11/04/2013 12:29 PM, Jiri Vanek wrote:
> > Imho "debug" should have nothing to do with "redirecting" also it
> > was used wrongly, and made consoel even less usefull...
One of the main motivation behind the console is to see stdout/stderr of
applets (especially on platforms where stdout/stderr is not visible by
default).
> +++ b/NEWS Mon Nov 04 16:08:16 2013 +0100
> +* Enabled and enhanced javaconsole for plugin and javaws
A slightly clearer to read version might be:
"A console for debugging plugin and javaws"
> +++ b/netx/net/sourceforge/jnlp/config/DeploymentConfiguration.java Mon Nov 04 16:08:16 2013 +0100
> @@ -64,14 +64,11 @@
> public static final String CONSOLE_HIDE = "HIDE";
> public static final String CONSOLE_SHOW = "SHOW";
> public static final String CONSOLE_DISABLE = "DISABLE";
> + public static final String CONSOLE_SHOW_PLUGIN = "SHOW_PLUGIN_ONLY";
> + public static final String CONSOLE_SHOW_JAVAWS = "SHOW_JAVAWS_ONLY";
FWIW, only SHOW, HIDE and DISABLE are documented for Oracle's
proprietary implementation. If you are going to add more, they should
be documented somewhere.
> @@ -160,6 +157,8 @@
> * Console
> */
> public static final String KEY_CONSOLE_STARTUP_MODE = "deployment.console.startup.mode";
> + //search for values above CONSOLE_*
Please put the comment before the variable. Adding it to the javadoc
should be fine too.
> +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties Mon Nov 04 16:08:16 2013 +0100
> +DPShowPluginOnly=Show on startup, only for plugin
"Show on plugin startup"
> +DPShowJavawsOnly=Show on startup, only for javaws
"Show on javaws startup"
> +DPJavaConsoleDisabledHint=Sorry, java console is disabled, turn it to hide, or show in itw-settings
Sorry, I am having trouble understanding this description. Maybe
something like this:
"Java console is disabled. Use itweb-settings to configure it to show on startup."
> +SPLASHmainL2 = You can get some more information by checking console, or even turn debug on in itw-settings
"Additional information may be available in the console or if debugging
is enabled"
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java Mon Nov 04 16:08:16 2013 +0100
> @@ -196,6 +193,16 @@
> + if (DeploymentConfiguration.CONSOLE_SHOW.equals(
> + JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_CONSOLE_STARTUP_MODE))
> + || (DeploymentConfiguration.CONSOLE_SHOW_PLUGIN.equals(
> + JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_CONSOLE_STARTUP_MODE))
> + && !isApplication)
> + || (DeploymentConfiguration.CONSOLE_SHOW_JAVAWS.equals(
> + JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_CONSOLE_STARTUP_MODE))
> + && isApplication)) {
> + JavaConsole.getConsole().showConsoleLater();
> + }
Is there a better place to put this code?
> +++ b/netx/net/sourceforge/jnlp/splashscreen/parts/JEditorPaneBasedExceptionDialog.java Mon Nov 04 16:08:16 2013 +0100
> @@ -166,9 +171,9 @@
> GroupLayout jPanel2Layout = new GroupLayout(topPanel);
> topPanel.setLayout(jPanel2Layout);
> jPanel2Layout.setHorizontalGroup(
> - jPanel2Layout.createParallelGroup(GroupLayout.Alignment.LEADING).addGroup(jPanel2Layout.createSequentialGroup().addContainerGap().addComponent(closeButton).addContainerGap().addComponent(aboutButton).addPreferredGap(LayoutStyle.ComponentPlacement.RELATED, 314, Short.MAX_VALUE).addComponent(closeAndCopyButton).addContainerGap()));
> + jPanel2Layout.createParallelGroup(GroupLayout.Alignment.LEADING).addGroup(jPanel2Layout.createSequentialGroup().addContainerGap().addComponent(closeButton).addContainerGap().addComponent(aboutButton).addContainerGap().addComponent(consoleButton).addPreferredGap(LayoutStyle.ComponentPlacement.RELATED, 314, Short.MAX_VALUE).addComponent(closeAndCopyButton).addContainerGap()));
> jPanel2Layout.setVerticalGroup(
> - jPanel2Layout.createParallelGroup(GroupLayout.Alignment.LEADING).addGroup(GroupLayout.Alignment.TRAILING, jPanel2Layout.createSequentialGroup().addContainerGap(24, Short.MAX_VALUE).addGroup(jPanel2Layout.createParallelGroup(GroupLayout.Alignment.BASELINE).addComponent(closeButton).addComponent(aboutButton).addComponent(closeAndCopyButton)).addContainerGap()));
> + jPanel2Layout.createParallelGroup(GroupLayout.Alignment.LEADING).addGroup(GroupLayout.Alignment.TRAILING, jPanel2Layout.createSequentialGroup().addContainerGap(24, Short.MAX_VALUE).addGroup(jPanel2Layout.createParallelGroup(GroupLayout.Alignment.BASELINE).addComponent(closeButton).addComponent(aboutButton).addComponent(consoleButton).addComponent(closeAndCopyButton)).addContainerGap()));
>
> exceptionLabel.setFont(new java.awt.Font("Dialog", 1, 18)); // NOI18N
> exceptionLabel.setHorizontalAlignment(SwingConstants.CENTER);
This is really, really hard to read. The convention, when writing trees
like this, is to do something like (code from another project):
GroupLayout groupLayout = new GroupLayout(getContentPane());
groupLayout.setHorizontalGroup(
groupLayout.createParallelGroup(Alignment.TRAILING)
.addGroup(groupLayout.createSequentialGroup()
.addContainerGap()
.addGroup(groupLayout.createParallelGroup(Alignment.TRAILING)
.addComponent(closeButton, GroupLayout.PREFERRED_SIZE, 92, GroupLayout.PREFERRED_SIZE)
.addComponent(panel, GroupLayout.DEFAULT_SIZE, 424, Short.MAX_VALUE))
.addContainerGap())
);
Basically, use indentation to 'show' the tree. Parallel branches go in
the same indentation level.
> +++ b/netx/net/sourceforge/jnlp/util/logging/JavaConsole.java Mon Nov 04 16:08:16 2013 +0100
There's lots of non-localized strings in this file. I am pointing them
out now, but feel free to localize them in a separate patch.
> + Copyright (C) 2009 Red Hat
Please update the copyright.
> + * A simple Java console for IcedTeaPlugin
Please update the comment.
> +public class JavaConsole {
> + * Initialize the console
> + */
> + private void initialize() {
> +
> + try {
> + UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
> + } catch (Exception e) {
> + OutputController.getLogger().log(OutputController.Level.ERROR_ALL,e);
> + }
This seems like the wrong place to set the look and feel. Isn't the look
and feel set up once in Launcher?
> + final String logDir = JNLPRuntime.getConfiguration().getProperty(DeploymentConfiguration.KEY_USER_LOG_DIR);
Unused variable?
Lots of missing localizations:
> + consoleWindow = new JDialog((JFrame)null, "Java Console");
> + JButton gcButton = new JButton("Run GC");
> + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, "Performing Garbage Collection....");
> + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, "Done");
> + JButton finalizersButton = new JButton("Run Finalizers");
> + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, "Running finalization....");
> + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, "Done");
> + JButton memoryButton = new JButton("Memory Info");
> + JButton systemPropertiesButton = new JButton("System Properties");
> + JButton classloadersButton = new JButton("Classloaders");
> + JButton threadListButton = new JButton("Thread List");
> + JButton closeButton = new JButton("Close");
> + public void showConsole() {
> + showConsole(false);
> + }
> +
> + public void showConsole(boolean b) {
> + consoleWindow.setModal(b);
> + consoleWindow.setVisible(true);
> + }
> +
> + public void hideConsole() {
> + consoleWindow.setModal(false);
> + consoleWindow.setVisible(false);
> + }
> +
> + public void showConsoleLater() {
> + showConsoleLater(false);
> + }
> + public void showConsoleLater(final boolean b) {
> + SwingUtilities.invokeLater(new Runnable() {
> + public void run() {
> + JavaConsole.getConsole().showConsole(b);
> + }
> + });
> + }
> +
> + public void hideConsoleLater() {
> + SwingUtilities.invokeLater(new Runnable() {
> + public void run() {
> + JavaConsole.getConsole().hideConsole();
> + }
> + });
> + }
This seems overly complex. Can you make it non-modal? And just
use showConsole() and hideConsole() ?
> +++ b/netx/net/sourceforge/jnlp/util/logging/OutputController.java Mon Nov 04 16:08:16 2013 +0100
> }
> + if (LogConfig.getLogConfig().isLogToConsole()) {
> + if (Level.isOutput(s)){
> + JavaConsole.getConsole().logOutput(message);
> + }
> + if (Level.isError(s)){
> + JavaConsole.getConsole().logError(message);
> + }
> + }
Cool, but this will not capture an applet's stdout/stderr, will it?
Thanks,
Omair
More information about the distro-pkg-dev
mailing list