[icedtea-web] resurrection of (output)console?

Tue Nov 5 07:12:03 PST 2013

* Jiri Vanek <jvanek at redhat.com> [2013-11-05 08:03]:
> On 11/04/2013 11:20 PM, Omair Majid wrote:
> >Comments in-line below.
> 
> You did not commented 1.4 version - this was not  new version of patch
> which obsolated the 1.4 but completely separate patch.

Ah. I see.

Is a significant change like this suitable for a 1.4.X update? I would
imagine the next 1.4 release would be bugfix only. Can we circle back to
the 1.4 patch after HEAD is sorted out?

> >* 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).
> 
> Yes, and exactly this it was never doing.

Really? I am sure this was working (at one point).

> I'm fully aware if it, and I hope this patch i leading to it. Also I
> have an intention to add also runtime plugin messages and also c++
> init messages before java side is even initialised (the initial
> Console impl have not even thought about it)

That's because the console is meant for applet developers. Not sure they
would care what the plugin itself is doing, as long as the applet is
working (and they can see any debugging output they write).

> >>+++ 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"
> 
> I'm happy to do so
> >
> >>+++ 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.
> 
> probably in wiki in 1.5 release notes, so it have time.

Please consider adding them as soon as the patch is pushed. Otherwise we
are likely to forget this.

> However I think they are quite self documenting

The documentation is for users. The code is self documenting, but I
don't think it's fair to expect users to read the code.

> >"Java console is disabled. Use itweb-settings to configure it to show on startup."
>   nearly - Java console is disabled. Use itweb-settings to configure it out of disabled to any show or hide value.
> 
> - as hide is ok here to (console showed on demand)

When is 'on demand'? I had to read it a few times before this version
made sense to me (and I am familiar with the implementation). Maybe
leave it not-exactly right to make it more obvious to users?

> >This seems like the wrong place to set the look and feel. Isn't the look
> >and feel set up once in Launcher?
> 
> You are whipping me for somebody else code :) This was hg move JavaConsole.

Just so we can both laugh at the joke, this is my code :D (And yes,
looking back at it, there's quite a few things I should have done
better).

Also, there's no whipping intended here....

> Tbh i do not know:( Anyway I'm going to work on console appearance so I would let this for that time.

I had it on my TODO list to i18n this correctly, but since you
volunteered to fix this up, I thought I should point it out. As I said,
you can fix this later :)

> >
> >>+++ 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?
> 
> not yet. But it never did.

Are you sure about this? JNLPRuntime would redirect streams to
java.stdout/java.stderr (we definitely know this was working) and the
console would read the file (see the TextAreaUpdater class) to display
stdout/stderr.

> But it *must* and is on todo:)
> 
> My current in mind design:

Please keep in mind that in mind that the main purpose of the Java
Console is to display System.out and System.err [1]. Additional
information (especially exceptions) are probably okay, but may not be
relevant to a developer developing a custom applet.

> +++ b/netx/net/sourceforge/jnlp/config/DeploymentConfiguration.java	Tue Nov 05 14:06:25 2013 +0100

> +    /**
> +     * when set to as value of KEY_CONSOLE_STARTUP_MODE = "deployment.console.startup.mode",
> +     * then console is not visible by default, but may be shown

Out of curiosity, how will it be shown? On an exception only?

>       */
>      public static final String CONSOLE_HIDE = "HIDE";

> +++ b/netx/net/sourceforge/jnlp/util/logging/JavaConsole.java	Tue Nov 05 14:06:25 2013 +0100

> +    public static boolean canShowOnStartup(boolean isApplication) {

Since the only place this is called is from JNLPRuntime, maybe you can
pass in the configuration object and avoid the repeated
JNLPRuntime.getConfiguration() calls?

> +        return 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);
> +    }

Thanks,
Omair

[1] http://docs.oracle.com/javase/7/docs/technotes/guides/deployment/deployment-guide/console.html