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

[Omair Majid]

On 11/26/2010 04:11 PM, Andrew Su wrote:
> 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.
>
> ChangeLog:
>
> 	* Makefile.am:
> 	(CONTROLPANEL_DIR): Build directory for control panel.
> 	(CONTROLPANEL_SRCDIR): Source directory for control panel.
> 	(CONTROLPANEL_LAUNCHER_OBJECTS): Objects used to compile binary
> 	control panel.
> 	(all-local): Add stamps/controlpanel-dist.stamp
> 	stamps/controlpanel.stamp $(CONTROLPANEL_DIR)/launcher/controlpanel.
> 	(clean-local): Add clean-controlpanel.
> 	(install-exec-local): Install the control panel jar file and control
> 	panel binary.
> 	(uninstall-local): Delete controlpanel.jar and controlpanel binary
> 	from installed location.
> 	(controlpanel-source-files.txt): Get the list of source files for
> 	control panel.
> 	(stamps/controlpanel.stamp): Compile the java files for control panel.
> 	(stamps/controlpanel-dist.stamp): Depend on stamps/controlpanel.stamp.
> 	Create the jar file for control panel.
> 	($(CONTROLPANEL_DIR)/launcher/%.o): Create the launcher objects.
> 	($(CONTROLPANEL_DIR)/launcher/controlpanel): Link the objects to make
> 	the launcher.
> 	(clean-controlpanel): Remove the compiled control panel.
> 	(controlpanel): Calls stamps/controlpanel.stamp.
> 	(controlpanel-dist): Calls stamps/controlpanel-dist.stamp.
> 	* controlpanel/org/classpath/controlpanel/AboutPanel.java,
> 	* controlpanel/org/classpath/controlpanel/ComboItem.java,
> 	* controlpanel/org/classpath/controlpanel/ControlPanel.java,
> 	* controlpanel/org/classpath/controlpanel/DebuggingPanel.java,
> 	* controlpanel/org/classpath/controlpanel/DesktopShortcutPanel.java,
> 	* controlpanel/org/classpath/controlpanel/JREPanel.java,
> 	* controlpanel/org/classpath/controlpanel/NamedBorderPanel.java,
> 	* controlpanel/org/classpath/controlpanel/SecuritySettingsPanel.java,
> 	* controlpanel/org/classpath/controlpanel/TemporaryInternetFilesPanel.java,
> 	* controlpanel/org/classpath/controlpanel/network/AdvancedProxySettingsDialog.java,
> 	* controlpanel/org/classpath/controlpanel/network/AdvancedProxySettingsPane.java,
> 	* controlpanel/org/classpath/controlpanel/network/NetworkSettingsPanel.java,
> 	* controlpanel/org/classpath/controlpanel/security/viewer/CertificatePane.java,
> 	* controlpanel/org/classpath/controlpanel/security/viewer/CertificateViewer.java,
> 	* controlpanel/org/classpath/controlpanel/translator/Translator.java:
> 	New classes. All methods are new as well.
> 	* netx/net/sourceforge/jnlp/resources/Messages.properties: Added
> 	messages for control panel.
>
> Comments? Question? Concerns?
>

A few quick thoughts are listed below. Please dont let any of that 
discourage you. These are all thing that should be improved. But what 
you have so far is great work. It's just not perfect yet ;) We really 
needed something like this in netx/plugin though. Thanks for doing this!

1. Some of these settings dont do anything yet, right? I am looking at 
the cache stuff here. Perhaps you might want to leave it out for now, or 
clearly indicate that it does not do anything.

2. Why does the binary have the name controlpanel? I dont really care 
about the name itself, but if you are going to make it so close to the 
one used by proprietary JDK, why not just call it ControlPanel?

3. We should look into adding a desktop file so users would be able to 
launch it from the menu.

4. It is customary to have the package names be representative of the 
url. IcedTea is under the url icedtea.classpath.org. So itt would make a 
lot more sense to use org.classpath.icedtea.* rather than org.classpath.*.

5. Please get rid of CertificateViewer. CertificatePanel should be moved 
to org.classpath.controlpanel, along with the rest of the classes.

6. Please dont create copies of classes. There is already a translator 
class in netx. The new Translator class still has a dependency on 
JNLPRuntime, so I really dont see the point of duplicating it. 
Personally I think it would be best if we separate the user-visible 
strings in ControlPanel from those in netx. We should have a separate 
properties file in control panel. Then the translator class would not be 
an exact duplicate (since it would read strings from another location).

7. Typo in the patch to ChangeLog:
+	(load): Do a security check at start. * security exception

8. Nice work on the launcher! The -Xbootclasspath way avoids adding 
redundant things to boot classpath.

9. I see that some files list asu as the author. Please use your full 
name (and email address) instead.

10. You know you can do static imports right? Something like
import static org.classpath.controlpanel.translator.Translator.R
would mean you can just do R("foo") instead of Translator.R everytime. 
This is more of a style thing, so feel free to ignore it.

Anyway, this looks great as a start! Thanks again for doing this.

Cheers,
Omair