[RFC][icedtea-web]: Adding control panel to icedtea-web

[Deepak Bhole]

* Andrew Su <asu at redhat.com> [2010-11-26 16:59]:
> Hello Deepak,
> 
> 
> ----- "Deepak Bhole" <dbhole at redhat.com> wrote:
> 
> > From: "Deepak Bhole" <dbhole at redhat.com>
> > To: "Andrew Su" <asu at redhat.com>
> > Cc: distro-pkg-dev at openjdk.java.net
> > Sent: Friday, November 26, 2010 4:28:17 PM GMT -05:00 US/Canada Eastern
> > Subject: Re: [RFC][icedtea-web]: Adding control panel to icedtea-web
> >
> > Hi Andrew,
> > 
> > * Andrew Su <asu at redhat.com> [2010-11-26 16:12]:
> > > Hello,
> > > 
> > > I have attached a patch to add a control panel for icedtea-web. This
> > can be used to modify deployments.properties. 
> > > 
> > > Current features:
> > >  - Enable/Disable caching.
> > >  - Set cache location.
> > >  - Set max space cache may use. (in MB)
> > >  - Set compression level of jar files.
> > >  - Import/View/Export/Remove certificates.
> > >  - Turn tracing on/off.
> > >  - Turn debugging on/off.
> > >  - Set behaviour of Java Console.
> > >  - Set whether to create desktop shortcut for javaws or not.
> > >  - Set proxy settings.
> > >  - Set how security warning behaviours.
> > > 
> > <snip>
> > > +
> > > +$(CONTROLPANEL_DIR)/launcher/%.o: $(LAUNCHER_SRCDIR)/%.c
> > > +	mkdir -p $(CONTROLPANEL_DIR)/launcher && \
> > > +	$(CC) $(LAUNCHER_FLAGS) -DJAVA_ARGS='{
> > "-Xbootclasspath/a:$(DESTDIR)$(prefix)/jre/lib/controlpanel.jar",
> > "-J-ms8m", "org.classpath.controlpanel.ControlPanel",  }'
> > -DPROGNAME="controlpanel" \
> > > +	-c -o $@ $<
> > > +
> > 
> > Why does the controlpanel jar need to be on the bootclasspath? Just
> > on
> > normal classpath should do..
> Using just classpath for the controlpanel will cause it to throw security exceptions from JNLPRuntime.initialize().
> 

Ah fair enough. Okay then we can leave it as bootclasspath.

> > 
> > > +$(CONTROLPANEL_DIR)/launcher/controlpanel:
> > $(CONTROLPANEL_LAUNCHER_OBJECTS)
> > > +	mkdir -p launcher
> > > +	$(CC) $(CONTROLPANEL_LAUNCHER_OBJECTS) $(LAUNCHER_LINK)
> > > +
> > <snip>
> > > +                        for (int j = 0; j <
> > javaConsoleItems.length; j++) {
> > > +                           
> > javaConsoleItems[j].setProperty("deployment.console.startup.mode");
> > 
> > 
> > I thought all property names had been stringified.. is that not the
> > case?
> This is to get the proper properties that this item in the combobox affects.
> Having it set the property is so that in the case we do say, add a new combobox we can reuse the listener without having to 
> add to that.
> 

No meant the property name string, "deployment.console.startup.mode" in
this case... I thought we had strings for each of the name, but  looking
at net/sourceforge/jnlp/runtime/DeploymentConfiguration.java, I see that
that is not the case. So nevermind in that case :)

> > 
> > > +                           
> > consoleComboBox.addItem(javaConsoleItems[j]);
> > > +                            if
> > (config.getProperty(javaConsoleItems[j].getProperty()).equals(javaConsoleItems[j].getValue()))
> > consoleComboBox.setSelectedIndex(j);
> > > +                        }
> > > +                        consoleComboBox.addItemListener(this);
> > > +                        add(consolePanel, c);
> > > +                }
> > > +
> > <snip>
> > > + *
> > > + */
> > > + at SuppressWarnings("serial")
> > > +public class SecuritySettingsPanel extends NamedBorderPanel
> > implements ActionListener {
> > > +
> > > +    private DeploymentConfiguration config;
> > > +
> > > +    // NOTE: All the ones listed with "Default" are in Oracle's
> > implementation.
> > > +    // Not shown on deployments.properties webpage. Add support for
> > these later!
> > > +    /** List of properties used by this panel */
> > > +    private String[] properties = {
> > "deployment.security.askgrantdialog.show",
> > > +            "deployment.security.askgrantdialog.notinca",
> > > +            "deployment.security.browser.keystore.use", // default
> > TRUE
> > > +            "deployment.security.clientauth.keystore.auto", //
> > Default FALSE
> > > +            "deployment.security.jsse.hostmismatch.warning",
> > > +            "deployment.security.https.warning.show", // Default
> > FALSE
> > > +            "deployment.security.sandbox.awtwarningwindow",
> > > +            "deployment.security.sandbox.jnlp.enhanced",
> > > +            "deployment.security.validation.crl", // Default TRUE
> > > +            "deployment.security.validation.ocsp", // Default
> > FALSE
> > > +            "deployment.security.pretrust.list", // Default TRUE
> > > +            "deployment.security.blacklist.check", // Default TRUE
> > > +            "deployment.security.password.cache", // Default TRUE
> > > +            "deployment.security.SSLv2Hello", // Default FALSE
> > > +            "deployment.security.SSLv3", // Default TRUE
> > > +            "deployment.security.TLSv1", // Default TRUE
> > > +//            "deployment.security.mixcode", // Default TRUE
> > > +    };
> > > +
> > 
> > The above should definitely be made static strings.
> > 
> > > +    public SecuritySettingsPanel(DeploymentConfiguration config) {
> > > +        super(Translator.R("CPHeadSecurity"), new BorderLayout());
> > > +        this.config = config;
> > > +
> > > +        addComponents();
> > > +    }
> > > +
> > 
> > The java files are also missing copyright headers. They need to be
> > added, and the dates for copied files (like Cert*java) needs to be
> > updated toi (i.e. make it 2008 -> 2008,2010).
> > 
> > Please also add more comments (atleast per class) stating what they
> > do.
> Will do these changes.
> 
> > 
> > Cheers,
> > Deepak
> 
> Thanks for looking it over.

Thank YOU for writing it! Nice work! :)

Deepak

> 
> Cheers,
>   Andrew