[rfc][icedtea-web] Remove need for HTML tag scanner in PluginAppletViewer

Jiri Vanek jvanek at redhat.com
Fri Oct 26 06:13:17 PDT 2012


On 10/25/2012 10:10 PM, Adam Domurad wrote:
> Hi all. This is something I've wanted to do for a while, simply because of the hackish nature of the
> applet/object/embed tag parsing code in ITW. Thoughts welcome, I'll still be doing some more testing
> on this (looks good so far) but would appreciate feedback. The patch also makes it possible to do
> unit tests from classes included in plugin.jar (used to unit test the new
> sun.java.PluginAppletAttributes).
>
> The applet tag information flows like this pre-patch:
> - NPAPI parses the tag information
> - ITW's C++ side takes in the already-parsed tag information, creates an embed tag (embed tag only)
> for ITW to use.
> - ITW's java side receives the tag, and scans it using (less than desirable) parsing routines.
>
> Post-patch:
> - NPAPI parses the tag information
> - ITW's C++ side generates a simple listing of the name value pairs passed
> - ITW's java side parses these name value pairs

I like this patch. I found several issues which had come across my mind, but I think during other 
reviews there will come more of them.
Also I'm not 100% capable to verify C code. SO before aprove I will definitely check with Pavel or 
somebody else can do this. I checked c part only from logical point of view.
>
> Points of contention:
> - PluginAppletViewer#parse had a 'ydisp' variable that has been changed to a static variable, since
> the parse method will now only ever handle one applet tag (the old version expected to have
> potentially multiple). However I'm not 100% about this because the old version as well only ever
> received one applet tag, rendering this effectively to always be 1... I'm not sure if the behaviour
> should be 'fixed' this way.

Your arguments seemed valid. And although PluginAppletViewer is singleton, making the ydisp variable 
static looks like it will behave differently. I would recommend empiric testing :)) See how multiple 
applets are shown on (multiple)page(s) and compare with your new implementation :)
>
> - The code was made to behave as-it-were as much as possible, meaning it can print a warning about a
> "missing code attribute in the embed tag" no matter what tag was used. This is as it was because ITW
> would always get passed an embed tag. Feel free to force me to change it:)
>
> ChangeLog:
> 2012-10-25  Adam Domurad <adomurad at redhat.com>
>
>      Remove the applet/embed/object tag parser from ITW. Send the applet
>      parameters directly from the C++.
>      * Makefile.am: Allow unit-testing for classes in plugin.jar.
>      * plugin/icedteanp/IcedTeaNPPlugin.cc: Send quoted parameter
>      name/values instead of applet tag. Remove some dead code.
>      * plugin/icedteanp/IcedTeaNPPlugin.h: Rename applet_tag ->
>      parameters_string.
>      * plugin/icedteanp/java/sun/applet/PluginAppletViewer.java:
>      Extract parsing code into its own class.
>      * plugin/icedteanp/java/sun/applet/PluginAppletAttributes.java:
>      New, encapsulates the (simplified) attribute parsing logic.
>      * tests/netx/unit/sun/applet/PluginAppletAttributesTest.java:
>      Unit tests for parsing logic.
>
>
> nuke-parser.patch
>
>
> diff --git a/Makefile.am b/Makefile.am
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -882,7 +882,7 @@ stamps/netx-unit-tests-compile.stamp: st
>   	mkdir -p $(NETX_UNIT_TEST_DIR) && \
>   	$(BOOT_DIR)/bin/javac $(IT_JAVACFLAGS) \
>   	 -d $(NETX_UNIT_TEST_DIR) \
> -	 -classpath $(JUNIT_JAR):$(NETX_DIR)/lib/classes.jar:$(TEST_EXTENSIONS_DIR) \
> +	 -classpath $(JUNIT_JAR):$(DESTDIR)$(datadir)/$(PACKAGE_NAME)/plugin.jar:$(NETX_DIR)/lib/classes.jar:$(TEST_EXTENSIONS_DIR) \
>   	 @netx-unit-tests-source-files.txt && \
>   	mkdir -p stamps && \
>   	touch $@
> @@ -912,7 +912,7 @@ stamps/run-netx-unit-tests.stamp: stamps
>   	done ; \
>   	cd $(NETX_UNIT_TEST_DIR) ; \
>   	class_names=`cat $(UNIT_CLASS_NAMES)` ; \
> -	CLASSPATH=$(NETX_DIR)/lib/classes.jar:$(JUNIT_JAR):$(JUNIT_RUNNER_JAR):$(TEST_EXTENSIONS_DIR):. \
> +	CLASSPATH=$(NETX_DIR)/lib/classes.jar:$(DESTDIR)$(datadir)/$(PACKAGE_NAME)/plugin.jar:$(JUNIT_JAR):$(JUNIT_RUNNER_JAR):$(TEST_EXTENSIONS_DIR):. \
>   	  $(BOOT_DIR)/bin/java -Xbootclasspath:$(RUNTIME) CommandLine $$class_names
>   if WITH_XSLTPROC
>   	$(XSLTPROC) $(TESTS_SRCDIR)/$(REPORT_STYLES_DIRNAME)/logs.xsl $(NETX_UNIT_TEST_DIR)/ServerAccess-logs.xml > $(TESTS_DIR)/logs_unit.html
> diff --git a/netx-dist-tests-whitelist b/netx-dist-tests-whitelist
> --- a/netx-dist-tests-whitelist
> +++ b/netx-dist-tests-whitelist
> @@ -1,1 +1,1 @@
> -.*
> +Applet.*
> diff --git a/plugin/icedteanp/IcedTeaNPPlugin.cc b/plugin/icedteanp/IcedTeaNPPlugin.cc
> --- a/plugin/icedteanp/IcedTeaNPPlugin.cc
> +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc
> @@ -229,8 +229,7 @@ static gboolean plugin_out_pipe_callback
>                                             GIOCondition condition,
>                                             gpointer plugin_data);
>   static NPError plugin_start_appletviewer (ITNPPluginData* data);
> -static gchar* plugin_create_applet_tag (int16_t argc, char* argn[],
> -                                        char* argv[]);
> +static std::string plugin_parameters_string (int argc, char* argn[], char* argv[]);
>   static void plugin_stop_appletviewer ();
>   // Uninitialize ITNPPluginData structure
>   static void plugin_data_destroy (NPP instance);
> @@ -347,7 +346,6 @@ ITNP_New (NPMIMEType pluginType, NPP ins
>
>     gchar* documentbase = NULL;
>     gchar* read_message = NULL;
> -  gchar* applet_tag = NULL;
>     gchar* cookie_info = NULL;
>
>     NPObject* npPluginObj = NULL;
> @@ -395,11 +393,10 @@ ITNP_New (NPMIMEType pluginType, NPP ins
>     documentbase = plugin_get_documentbase (instance);
>     if (documentbase && argc != 0)
>       {
> -      // Send applet tag message to appletviewer.
> -      applet_tag = plugin_create_applet_tag (argc, argn, argv);
> -
> -      data->applet_tag = (gchar*) malloc(strlen(applet_tag)*sizeof(gchar) + strlen(documentbase)*sizeof(gchar) + 32);
> -      g_sprintf(data->applet_tag, "tag %s %s", documentbase, applet_tag);
> +      // Send parameters to appletviewer.
> +      std::string params_string = plugin_parameters_string(argc, argn, argv);
> +
> +      data->parameters_string =  g_strdup_printf("tag %s %s", documentbase, params_string.c_str());
>
>         data->is_applet_instance = true;
>       }
> @@ -424,33 +421,7 @@ ITNP_New (NPMIMEType pluginType, NPP ins
>
>     instance->pdata = data;
>
> -  goto cleanup_done;
> -
> - cleanup_appletviewer_mutex:
> -  g_mutex_free (data->appletviewer_mutex);
> -  data->appletviewer_mutex = NULL;
> -
> -  // cleanup_instance_string:
> -  g_free (data->instance_id);
> -  data->instance_id = NULL;
> -
> -  // cleanup applet tag:
> -  g_free (data->applet_tag);
> -  data->applet_tag = NULL;
> -
> -  // cleanup_data:
> -  // Eliminate back-pointer to plugin instance.
> -  data->owner = NULL;
> -  (*browser_functions.memfree) (data);
> -  data = NULL;
> -
> -  // Initialization failed so return a NULL pointer for the browser
> -  // data.
> -  instance->pdata = NULL;
> -
>    cleanup_done:
> -  g_free (applet_tag);
> -  applet_tag = NULL;
>     g_free (read_message);
>     read_message = NULL;
>     g_free (documentbase);
> @@ -834,7 +805,7 @@ ITNP_SetWindow (NPP instance, NPWindow*
>         // Now we have everything. Send this data to the Java side
>         plugin_send_initialization_message(
>       		  data->instance_id, (gulong) data->window_handle,
> -    		  data->window_width, data->window_height, data->applet_tag);
> +    		  data->window_width, data->window_height, data->parameters_string);
>
>         g_mutex_unlock (data->appletviewer_mutex);
>
> @@ -1696,157 +1667,66 @@ plugin_start_appletviewer (ITNPPluginDat
>
>   /*
>    * Replaces certain characters (\r, \n, etc) with HTML escape equivalents.
> - *
> - * Return string is allocated on the heap. Caller assumes responsibility
> - * for freeing the memory via free()
>    */
> -static char*
> -encode_string(char* to_encode)
> -{
> -
> -  // Do nothing for an empty string
> -  if (to_encode == '\0')
> -      return to_encode;
> -
> -  // worst case scenario -> all characters are newlines or
> -  // returns, each of which translates to 5 substitutions
> -  char* encoded = (char*) calloc(((strlen(to_encode)*5)+1), sizeof(char));
> -
> -  strcpy(encoded, "");
> -
> -  for (int i=0; i < strlen(to_encode); i++)
> +static std::string
> +encode_string(const char* to_encode) {
> +  std::string encoded;
> +
> +  if (to_encode == NULL)
> +  {
> +      return encoded;
> +  }
> +
> +  size_t length = strlen(to_encode);
> +  for (int i=0; i < length; i++)
>     {
>         if (to_encode[i] == '\r')
> -          encoded = strcat(encoded, "&#13;");
> +          encoded += "&#13;";
>         else if (to_encode[i] == '\n')
> -          encoded = strcat(encoded, "&#10;");
> +          encoded += "&#10;";
>         else if (to_encode[i] == '>')
> -          encoded = strcat(encoded, "&gt;");
> +    	  encoded += "&gt;";
>         else if (to_encode[i] == '<')
> -          encoded = strcat(encoded, "&lt;");
> +    	  encoded += "&lt;";
>         else if (to_encode[i] == '&')
> -          encoded = strcat(encoded, "&amp;");
> +    	  encoded += "&amp;";
>         else if (to_encode[i] == '"')
> -          encoded = strcat(encoded, "&quot;");
> +    	  encoded += "&quot;";
>         else
> -      {
> -           char* orig_char = (char*) calloc(2, sizeof(char));
> -           orig_char[0] = to_encode[i];
> -           orig_char[1] = '\0';
> -
> -           strcat(encoded, orig_char);
> -
> -           free(orig_char);
> -           orig_char = NULL;
> -      }
> +          encoded += to_encode[i];
>     }
>
>     return encoded;

This is probably strangest stuff in patch. NPAPI is returning already decoded strings. Why to encode 
them again?!?!?!

Unless I see something very wrong, get rid of this unnecessary middle step!

>   }
>
> -// Build up the applet tag string that we'll send to the applet
> -// viewer.
> -static gchar*
> -plugin_create_applet_tag (int16_t argc, char* argn[], char* argv[])
> +// Build a string containing an encoded list of parameters to send to
> +// the applet viewer
> +static std::string
> +plugin_parameters_string (int argc, char* argn[], char* argv[])
>   {
> -  PLUGIN_DEBUG ("plugin_create_applet_tag\n");
> -
> -  gchar* applet_tag = g_strdup ("<EMBED ");
> -  gchar* parameters = g_strdup ("");
> -
> -  for (int16_t i = 0; i < argc; i++)
> +  PLUGIN_DEBUG ("plugin_parameters_string\n");
> +
> +  std::string parameters;
> +
> +  for (int i = 0; i < argc; i++)
> +  {
> +    if (argv[i] != NULL)
>       {
> -      gchar* argn_escaped = encode_string(argn[i]);
> -      gchar* argv_escaped = encode_string(argv[i]);
> -
> -      if (!g_ascii_strcasecmp (argn_escaped, "code"))
> -        {
> -          gchar* code = g_strdup_printf ("CODE=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, code, NULL);
> -          g_free (code);
> -          code = NULL;
> +        std::string name_escaped = encode_string(argn[i]);
> +        std::string value_escaped = encode_string(argv[i]);
> +
> +        //Encode parameters and send as '"name1" "value1" "name2" "value2"' etc
> +        parameters += "\"";
> +        parameters += name_escaped;
> +        parameters += "\" ";
> +        parameters += value_escaped;
> +        parameters += "\" ";
>       }
> -      else if (!g_ascii_strcasecmp (argn_escaped, "java_code"))
> -    {
> -          gchar* java_code = g_strdup_printf ("JAVA_CODE=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, java_code, NULL);
> -          g_free (java_code);
> -          java_code = NULL;
> -    }
> -      else if (!g_ascii_strcasecmp (argn_escaped, "codebase"))
> -    {
> -          gchar* codebase = g_strdup_printf ("CODEBASE=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, codebase, NULL);
> -          g_free (codebase);
> -          codebase = NULL;
> -    }
> -      else if (!g_ascii_strcasecmp (argn_escaped, "java_codebase"))
> -    {
> -          gchar* java_codebase = g_strdup_printf ("JAVA_CODEBASE=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, java_codebase, NULL);
> -          g_free (java_codebase);
> -          java_codebase = NULL;
> -    }
> -      else if (!g_ascii_strcasecmp (argn_escaped, "classid"))
> -    {
> -          gchar* classid = g_strdup_printf ("CLASSID=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, classid, NULL);
> -          g_free (classid);
> -          classid = NULL;
> -    }
> -      else if (!g_ascii_strcasecmp (argn_escaped, "archive"))
> -    {
> -          gchar* archive = g_strdup_printf ("ARCHIVE=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, archive, NULL);
> -          g_free (archive);
> -          archive = NULL;
> -    }
> -      else if (!g_ascii_strcasecmp (argn_escaped, "java_archive"))
> -    {
> -          gchar* java_archive = g_strdup_printf ("JAVA_ARCHIVE=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, java_archive, NULL);
> -          g_free (java_archive);
> -          java_archive = NULL;
> -    }
> -      else if (!g_ascii_strcasecmp (argn_escaped, "width"))
> -    {
> -          gchar* width = g_strdup_printf ("width=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, width, NULL);
> -          g_free (width);
> -          width = NULL;
> -    }
> -      else if (!g_ascii_strcasecmp (argn_escaped, "height"))
> -    {
> -          gchar* height = g_strdup_printf ("height=\"%s\" ", argv_escaped);
> -          applet_tag = g_strconcat (applet_tag, height, NULL);
> -          g_free (height);
> -          height = NULL;
> -    }
> -      else
> -        {
> -
> -          if (argv_escaped != '\0')
> -            {
> -              parameters = g_strconcat (parameters, "<PARAM NAME=\"", argn_escaped,
> -                                        "\" VALUE=\"", argv_escaped, "\">", NULL);
> -            }
> -        }
> -
> -      free(argn_escaped);
> -      free(argv_escaped);
> -
> -      argn_escaped = NULL;
> -      argv_escaped = NULL;
> -    }
> -
> -  applet_tag = g_strconcat (applet_tag, ">", parameters, "</EMBED>", NULL);
> -
> -  g_free (parameters);
> -  parameters = NULL;
> -
> -  PLUGIN_DEBUG ("plugin_create_applet_tag return\n");
> -
> -  return applet_tag;
> +  }
> +
> +  PLUGIN_DEBUG ("plugin_parameters_string return\n");
> +
> +  return parameters;
>   }
>
>   // plugin_send_message_to_appletviewer must be called while holding
> @@ -2057,8 +1937,8 @@ plugin_data_destroy (NPP instance)
>     tofree->instance_id = NULL;
>
>     // cleanup applet tag
> -  g_free (tofree->applet_tag);
> -  tofree->applet_tag = NULL;
> +  g_free (tofree->parameters_string);
> +  tofree->parameters_string = NULL;
>
>     g_free(tofree->source);
>     tofree->source = NULL;
> @@ -2537,7 +2417,7 @@ get_scriptable_object(NPP instance)
>           // a 0 handle
>           if (!data->window_handle)
>           {
> -            plugin_send_initialization_message(data->instance_id, 0, 0, 0, data->applet_tag);
> +            plugin_send_initialization_message(data->instance_id, 0, 0, 0, data->parameters_string);
>           }
>
>           java_result = java_request.getAppletObjectInstance(id_str);
> diff --git a/plugin/icedteanp/IcedTeaNPPlugin.h b/plugin/icedteanp/IcedTeaNPPlugin.h
> --- a/plugin/icedteanp/IcedTeaNPPlugin.h
> +++ b/plugin/icedteanp/IcedTeaNPPlugin.h
> @@ -66,8 +66,8 @@ struct ITNPPluginData
>   {
>     // A unique identifier for this plugin window.
>     gchar* instance_id;
> -  // The applet tag sent to Java side
> -  gchar* applet_tag;
> +  // The parameter list string sent to Java side
> +  gchar* parameters_string;
>     // Mutex to protect appletviewer_alive.
>     GMutex* appletviewer_mutex;
>     // Back-pointer to the plugin instance to which this data belongs.
> diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletAttributes.java b/plugin/icedteanp/java/sun/applet/PluginAppletAttributes.java
> new file mode 100644
> --- /dev/null
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletAttributes.java
> @@ -0,0 +1,169 @@
> +/* PluginAppletAttributes -- Provides parsing for applet attributes
> +   Copyright (C) 2012  Red Hat
> +
> +This file is part of IcedTea.
> +
> +IcedTea is free software; you can redistribute it and/or modify
> +it under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 2, or (at your option)
> +any later version.
> +
> +IcedTea is distributed in the hope that it will be useful, but
> +WITHOUT ANY WARRANTY; without even the implied warranty of
> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License
> +along with IcedTea; see the file COPYING.  If not, write to the
> +Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +02110-1301 USA.
> +
> +Linking this library statically or dynamically with other modules is
> +making a combined work based on this library.  Thus, the terms and
> +conditions of the GNU General Public License cover the whole
> +combination.
> +
> +As a special exception, the copyright holders of this library give you
> +permission to link this library with independent modules to produce an
> +executable, regardless of the license terms of these independent
> +modules, and to copy and distribute the resulting executable under
> +terms of your choice, provided that you also meet, for each linked
> +independent module, the terms and conditions of the license of that
> +module.  An independent module is a module which is not derived from
> +or based on this library.  If you modify this library, you may extend
> +this exception to your version of the library, but you are not
> +obligated to do so.  If you do not wish to do so, delete this
> +exception statement from your version. */
> +
> +package sun.applet;
> +
> +import java.io.PrintStream;
> +import java.util.ArrayList;
> +import java.util.Enumeration;
> +import java.util.Hashtable;
> +import java.util.List;
> +
> +/**
> + * Provides parsing for applet attributes passed from the C++ side as quoted,
> + * encoded strings. This class is not meant to be initialized

No author in ITW sources please:)

Please, mke  this class as classical object. You can still keep some static (or singleton) utility 
methods here, but prefer more object way:

  Hashtable<String, String> atts = PluginAppletAttributes.getPArser().parse(width, height, tag);
or
  Hashtable<String, String> atts = new PluginAppletAttributes().parse(width, height, tag);

Although I would like to recomand to wrap the Hashtable<String, String> to Object. Eg 
ParsedAppletTag. so the above methods wirr return this one.
One will thenbe able to access parsedAppletTag.getKey() and will be also able to recieve something 
else then Strings

And again. Get rid of redundant decoding!


> + * @author Adam Domurad
> + */
> +class PluginAppletAttributes {
> +    /**
> +     * Decodes the string (converts html escapes into proper characters)
> +     *
> +     * @param toDecode The string to decode
> +     * @return The decoded string
> +     */
> +    static String decodeString(String toDecode) {
> +
> +        toDecode = toDecode.replace("&gt;", ">");
> +        toDecode = toDecode.replace("&lt;", "<");
> +        toDecode = toDecode.replace("&#10;", "\n");
> +        toDecode = toDecode.replace("&#13;", "\r");
> +        toDecode = toDecode.replace("&quot;", "\"");
> +        toDecode = toDecode.replace("&amp;", "&");
> +
> +        return toDecode;
> +    }
> +
> +    static final boolean isInt(String s) {
> +        try {
> +            Integer.parseInt(s);
> +            return true;
> +        } catch (NumberFormatException e) {
> +            return false;
> +        }
> +    }

:DDD no! :D Although this give sense, I would not recommand usage of exception for directing the 
flow of code :)
My suggestion will be matches \d+ ;)
I have also some susspiccion that there is ssDigit method somewhere.

> +
> +    static List<String> extractQuotedStrings(String s) {
> +        List<String> strs = new ArrayList<String>();
> +        int idx = 0;
> +
> +        while (idx < s.length()){
> +            // Move to character after starting quote
> +            idx = s.indexOf('"', idx) + 1;
> +            if (idx <= 0) break;
> +
> +            // Find end quote
> +            int endIdx = s.indexOf('"', idx);
> +            strs.add(s.substring(idx, endIdx));
> +            idx = endIdx + 1;
> +        }
> +
> +        return strs;
> +    }
> +
> +    static Hashtable<String, String> parseQuotedKeyValuePairs(String keyvalString) {
> +        List<String> strs = extractQuotedStrings(keyvalString);
> +        Hashtable<String, String> attributes = new Hashtable<String,String>();
> +
> +        for (int i = 0; i < strs.size(); i += 2) {
> +            String key = decodeString(strs.get(i)).toLowerCase();
> +            String value = decodeString(strs.get(i + 1));
> +            attributes.put(key, value);
> +        }
> +
> +        return attributes;
> +    }
> +
> +    /**
> +     * Generates a hashtable of applet attributes, given a string containing
> +     * parameters in quotes.
> +     *
> +     * @param width default applet width
> +     * @param height default applet height
> +     * @param parameterString the parameters
> +     * @return the attributes in a hash table
> +     */
> +    static public Hashtable<String, String> parse(String width,
> +            String height, String parameterString) {
> +        Hashtable<String, String> atts = parseQuotedKeyValuePairs(parameterString);
> +
> +        // If there is a classid and no code tag present, transform it to code tag
> +        if (atts.get("code") == null && atts.get("classid") != null
> +                && !(atts.get("classid")).startsWith("clsid:")) {
> +            atts.put("code", atts.get("classid"));
> +        }
> +
> +        // remove java: from code tag
> +        if (atts.get("code") != null && (atts.get("code")).startsWith("java:")) {
> +            atts.put("code", (atts.get("code")).substring(5));
> +        }
> +
> +        // java_* aliases override older names:
> +        //http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#in-nav
> +        if (atts.get("java_code") != null) {
> +            atts.put("code", (atts.get("java_code")));
> +        }
> +
> +        if (atts.get("java_codebase") != null) {
> +            atts.put("codebase", (atts.get("java_codebase")));
> +        }
> +
> +        if (atts.get("java_archive") != null) {
> +            atts.put("archive", (atts.get("java_archive")));
> +        }
> +
> +        if (atts.get("java_object") != null) {
> +            atts.put("object", (atts.get("java_object")));
> +        }
> +
> +        if (atts.get("java_type") != null) {
> +            atts.put("type", (atts.get("java_type")));
> +        }
> +
> +        if (atts.get("width") == null || !isInt(atts.get("width"))) {
> +            atts.put("width", width);
> +        }
> +
> +        if (atts.get("height") == null || !isInt(atts.get("height"))) {
> +            atts.put("height", height);
> +        }
> +
> +        if (atts.get("code") == null && atts.get("object") == null) {
> +            return null;
> +        }
> +        return atts;
> +    }
> +}
> diff --git a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> --- a/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> +++ b/plugin/icedteanp/java/sun/applet/PluginAppletViewer.java
> @@ -86,6 +86,7 @@ import java.lang.reflect.InvocationTarge
>   import java.net.SocketPermission;
>   import java.net.URI;
>   import java.net.URL;
> +import java.net.URLConnection;
>   import java.security.AccessController;
>   import java.security.AllPermission;
>   import java.security.PrivilegedAction;
> @@ -286,6 +287,14 @@ public class PluginAppletViewer extends
>           PRE_INIT, INIT_COMPLETE, REFRAME_COMPLETE, INACTIVE, DESTROYED
>       };
>
> +    /* values used for placement of AppletViewer's frames */
> +    private static final int XDELTA = 30;
> +    private static final int YDELTA = XDELTA;
> +
> +    private static int x = 0;
> +    private static int y = 0;
> +    private static int ydisp = 1;
> +
>       /**
>        * The panel in which the applet is being displayed.
>        */
> @@ -488,10 +497,37 @@ public class PluginAppletViewer extends
>                                       "DocumentBase = ", documentBase, "\n",
>                                       "Tag = ", tag);
>
> -                PluginAppletViewer.parse
> -                                        (identifier, handle, width, height,
> -                                                new StringReader(tag),
> -                                                new URL(documentBase));
> +                PluginAppletPanelFactory factory = new PluginAppletPanelFactory();
> +                AppletMessageHandler amh = new AppletMessageHandler("appletviewer");
> +                URL url = new URL(documentBase);
> +                URLConnection conn = url.openConnection();
> +                /* The original URL may have been redirected - this
> +                 * sets it to whatever URL/codebase we ended up getting
> +                 */
> +                url = conn.getURL();
> +

I would like to see thios in different method. Also the ydisp remains questionable. Imho with 
yournew approach it will lead to complete removal of it (i'm really afraid of case  like three 
applets per page, and several such a pages)

> +                Hashtable<String, String> atts = PluginAppletAttributes.parse(width, height, tag);
> +
> +                if (atts == null) {
> +                    String embedRequiresCode = amh.getMessage("parse.warning.embed.requirescode");
> +                    System.out.println(embedRequiresCode);
> +
> +                    throw new RuntimeException(embedRequiresCode);
> +                }
> +
> +                // Let user know we are starting up
> +                streamhandler.write("instance " + identifier + " status " + amh.getMessage("status.start"));
> +                factory.createPanel(streamhandler, identifier, handle, x, y, url, atts);
> +
> +                x += XDELTA;
> +                y += YDELTA;
> +                // make sure we don't go too far!
> +                Dimension d = Toolkit.getDefaultToolkit().getScreenSize();
> +                if ((x > d.width - 300) || (y > d.height - 300)) {
> +                    x = 0;
> +                    y = 2 * ydisp * YDELTA;
> +                    ydisp++;
> +                }
>
>                   long maxTimeToSleep = APPLET_TIMEOUT;
>                   appletsLock.lock();
> @@ -1423,24 +1459,6 @@ public class PluginAppletViewer extends
>       }
>
>       /**
> -     * Decodes the string (converts html escapes into proper characters)
> -     *
> -     * @param toDecode The string to decode
> -     * @return The decoded string
> -     */
> -    public static String decodeString(String toDecode) {
> -
> -        toDecode = toDecode.replace("&gt;", ">");
> -        toDecode = toDecode.replace("&lt;", "<");
> -        toDecode = toDecode.replace("&amp;", "&");
> -        toDecode = toDecode.replace("&#10;", "\n");
> -        toDecode = toDecode.replace("&#13;", "\r");
> -        toDecode = toDecode.replace("&quot;", "\"");
> -
> -        return toDecode;
> -    }
> -
> -    /**
>        * System parameters.
>        */
>       static Hashtable<String, String> systemParam = new Hashtable<String, String>();
> @@ -1457,67 +1475,6 @@ public class PluginAppletViewer extends
>       }
>
>       /**
> -     * Print the HTML tag.
> -     */
> -    public static void printTag(PrintStream out, Hashtable<String, String> atts) {
> -        out.print("<applet");
> -
> -        String v = atts.get("codebase");
> -        if (v != null) {
> -            out.print(" codebase=\"" + v + "\"");
> -        }
> -
> -        v = atts.get("code");
> -        if (v == null) {
> -            v = "applet.class";
> -        }
> -        out.print(" code=\"" + v + "\"");
> -        v = atts.get("width");
> -        if (v == null) {
> -            v = "150";
> -        }
> -        out.print(" width=" + v);
> -
> -        v = atts.get("height");
> -        if (v == null) {
> -            v = "100";
> -        }
> -        out.print(" height=" + v);
> -
> -        v = atts.get("name");
> -        if (v != null) {
> -            out.print(" name=\"" + v + "\"");
> -        }
> -        out.println(">");
> -
> -        // A very slow sorting algorithm
> -        int len = atts.size();
> -        String params[] = new String[len];
> -        len = 0;
> -        for (Enumeration<String> e = atts.keys(); e.hasMoreElements();) {
> -            String param = e.nextElement();
> -            int i = 0;
> -            for (; i < len; i++) {
> -                if (params[i].compareTo(param) >= 0) {
> -                    break;
> -                }
> -            }
> -            System.arraycopy(params, i, params, i + 1, len - i);
> -            params[i] = param;
> -            len++;
> -        }
> -
> -        for (int i = 0; i < len; i++) {
> -            String param = params[i];
> -            if (systemParam.get(param) == null) {
> -                out.println("<param name=" + param +
> -                        " value=\"" + atts.get(param) + "\">");
> -            }
> -        }
> -        out.println("</applet>");
> -    }
> -
> -    /**
>        * Make sure the atrributes are uptodate.
>        */
>       public void updateAtts() {
> @@ -1673,412 +1630,6 @@ public class PluginAppletViewer extends
>           return appletPanels.size();
>       }
>
> -    /**
> -     * Scan spaces.
> -     */
> -    public static void skipSpace(int[] c, Reader in) throws IOException {
> -        while ((c[0] >= 0) &&
> -                ((c[0] == ' ') || (c[0] == '\t') || (c[0] == '\n') || (c[0] == '\r'))) {
> -            c[0] = in.read();
> -        }
> -    }
> -
> -    /**
> -     * Scan identifier
> -     */
> -    public static String scanIdentifier(int[] c, Reader in) throws IOException {
> -        StringBuilder buf = new StringBuilder();
> -
> -        if (c[0] == '!') {
> -            // Technically, we should be scanning for '!--' but we are reading
> -            // from a stream, and there is no way to peek ahead. That said,
> -            // a ! at this point can only mean comment here afaik, so we
> -            // should be okay
> -            skipComment(c, in);
> -            return "";
> -        }
> -
> -        while (true) {
> -            if (((c[0] >= 'a') && (c[0] <= 'z')) ||
> -                    ((c[0] >= 'A') && (c[0] <= 'Z')) ||
> -                    ((c[0] >= '0') && (c[0] <= '9')) || (c[0] == '_')) {
> -                buf.append((char) c[0]);
> -                c[0] = in.read();
> -            } else {
> -                return buf.toString();
> -            }
> -        }
> -    }
> -
> -    public static void skipComment(int[] c, Reader in) throws IOException {
> -        StringBuilder buf = new StringBuilder();
> -        boolean commentHeaderPassed = false;
> -        c[0] = in.read();
> -        buf.append((char) c[0]);
> -
> -        while (true) {
> -            if (c[0] == '-' && (c[0] = in.read()) == '-') {
> -                buf.append((char) c[0]);
> -                if (commentHeaderPassed) {
> -                    // -- encountered ... is > next?
> -                    if ((c[0] = in.read()) == '>') {
> -                        buf.append((char) c[0]);
> -
> -                        PluginDebug.debug("Comment skipped: ", buf.toString());
> -
> -                        // comment skipped.
> -                        return;
> -                    }
> -                } else {
> -                    // first -- is part of <!-- ... , just mark that we have passed it
> -                    commentHeaderPassed = true;
> -                }
> -
> -            } else if (commentHeaderPassed == false) {
> -                buf.append((char) c[0]);
> -                PluginDebug.debug("Warning: Attempted to skip comment, but this tag does not appear to be a comment: ", buf.toString());
> -                return;
> -            }
> -
> -            c[0] = in.read();
> -            buf.append((char) c[0]);
> -        }
> -    }
> -
> -    /**
> -     * Scan tag
> -     */
> -    public static Hashtable<String, String> scanTag(int[] c, Reader in) throws IOException {
> -        Hashtable<String, String> atts = new Hashtable<String, String>();
> -        skipSpace(c, in);
> -        while (c[0] >= 0 && c[0] != '>') {
> -            String att = decodeString(scanIdentifier(c, in));
> -            String val = "";
> -            skipSpace(c, in);
> -            if (c[0] == '=') {
> -                int quote = -1;
> -                c[0] = in.read();
> -                skipSpace(c, in);
> -                if ((c[0] == '\'') || (c[0] == '\"')) {
> -                    quote = c[0];
> -                    c[0] = in.read();
> -                }
> -                StringBuilder buf = new StringBuilder();
> -                while ((c[0] > 0) &&
> -                        (((quote < 0) && (c[0] != ' ') && (c[0] != '\t') &&
> -                                (c[0] != '\n') && (c[0] != '\r') && (c[0] != '>'))
> -                         || ((quote >= 0) && (c[0] != quote)))) {
> -                    buf.append((char) c[0]);
> -                    c[0] = in.read();
> -                }
> -                if (c[0] == quote) {
> -                    c[0] = in.read();
> -                }
> -                skipSpace(c, in);
> -                val = decodeString(buf.toString());
> -            }
> -
> -            PluginDebug.debug("PUT ", att, " = '", val, "'");
> -            atts.put(att.toLowerCase(java.util.Locale.ENGLISH), val);
> -
> -            while (true) {
> -                if ((c[0] == '>') || (c[0] < 0) ||
> -                        ((c[0] >= 'a') && (c[0] <= 'z')) ||
> -                        ((c[0] >= 'A') && (c[0] <= 'Z')) ||
> -                        ((c[0] >= '0') && (c[0] <= '9')) || (c[0] == '_'))
> -                    break;
> -                c[0] = in.read();
> -            }
> -            //skipSpace(in);
> -        }
> -        return atts;
> -    }
> -
> -    // private static final == inline
> -    private static final boolean isInt(Object o) {
> -        boolean isInt = false;
> -        try {
> -            Integer.parseInt((String) o);
> -            isInt = true;
> -        } catch (Exception e) {
> -            // don't care
> -        }
> -
> -        return isInt;
> -    }
> -
> -    /* values used for placement of AppletViewer's frames */
> -    private static int x = 0;
> -    private static int y = 0;
> -    private static final int XDELTA = 30;
> -    private static final int YDELTA = XDELTA;
> -
> -    static String encoding = null;
> -
> -    /**
> -     * Scan an html file for <applet> tags
> -     */
> -    public static void parse(int identifier, long handle, String width, String height, Reader in, URL url, String enc)
> -            throws IOException {
> -        encoding = enc;
> -        parse(identifier, handle, width, height, in, url, System.out, new PluginAppletPanelFactory());
> -    }
> -
> -    public static void parse(int identifier, long handle, String width, String height, Reader in, URL url)
> -            throws PrivilegedActionException {
> -
> -        final int fIdentifier = identifier;
> -        final long fHandle = handle;
> -        final String fWidth = width;
> -        final String fHeight = height;
> -        final Reader fIn = in;
> -        final URL fUrl = url;
> -        AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
> -            public Void run() throws IOException {
> -                parse(fIdentifier, fHandle, fWidth, fHeight, fIn, fUrl,
> -                        System.out, new PluginAppletPanelFactory());
> -                return null;
> -            }
> -        });
> -    }
> -
> -    @SuppressWarnings("unused")
> -    public static void parse(int identifier, long handle, String width,
> -                 String height, Reader in, URL url,
> -                              PrintStream statusMsgStream,
> -                              PluginAppletPanelFactory factory)
> -            throws IOException {
> -        boolean isObjectTag = false;
> -        boolean objectTagAlreadyParsed = false;
> -
> -        // The current character
> -        // FIXME: This is an evil hack to force pass-by-reference.. the
> -        // parsing code needs to be rewritten from scratch to prevent such
> -        //a need
> -        int[] c = new int[1];
> -
> -        // warning messages
> -        String requiresNameWarning = amh.getMessage("parse.warning.requiresname");
> -        String paramOutsideWarning = amh.getMessage("parse.warning.paramoutside");
> -        String appletRequiresCodeWarning = amh.getMessage("parse.warning.applet.requirescode");
> -        String appletRequiresHeightWarning = amh.getMessage("parse.warning.applet.requiresheight");
> -        String appletRequiresWidthWarning = amh.getMessage("parse.warning.applet.requireswidth");
> -        String objectRequiresCodeWarning = amh.getMessage("parse.warning.object.requirescode");
> -        String objectRequiresHeightWarning = amh.getMessage("parse.warning.object.requiresheight");
> -        String objectRequiresWidthWarning = amh.getMessage("parse.warning.object.requireswidth");
> -        String embedRequiresCodeWarning = amh.getMessage("parse.warning.embed.requirescode");
> -        String embedRequiresHeightWarning = amh.getMessage("parse.warning.embed.requiresheight");
> -        String embedRequiresWidthWarning = amh.getMessage("parse.warning.embed.requireswidth");
> -        String appNotLongerSupportedWarning = amh.getMessage("parse.warning.appnotLongersupported");
> -
> -        java.net.URLConnection conn = url.openConnection();
> -        /* The original URL may have been redirected - this
> -         * sets it to whatever URL/codebase we ended up getting
> -         */
> -        url = conn.getURL();
> -
> -        int ydisp = 1;
> -        Hashtable<String, String> atts = null;
> -
> -        while (true) {
> -            c[0] = in.read();
> -            if (c[0] == -1)
> -                break;
> -
> -            if (c[0] == '<') {
> -                c[0] = in.read();
> -                if (c[0] == '/') {
> -                    c[0] = in.read();
> -                    String nm = scanIdentifier(c, in);
> -                    if (nm.equalsIgnoreCase("applet") ||
> -                             nm.equalsIgnoreCase("object") ||
> -                             nm.equalsIgnoreCase("embed")) {
> -
> -                        // We can't test for a code tag until </OBJECT>
> -                        // because it is a parameter, not an attribute.
> -                        if (isObjectTag) {
> -                            if (atts.get("code") == null && atts.get("object") == null) {
> -                                statusMsgStream.println(objectRequiresCodeWarning);
> -                                atts = null;
> -                            }
> -                        }
> -
> -                        if (atts != null) {
> -                            // XXX 5/18 In general this code just simply
> -                            // shouldn't be part of parsing.  It's presence
> -                            // causes things to be a little too much of a
> -                            // hack.
> -
> -                            // Let user know we are starting up
> -                            streamhandler.write("instance " + identifier + " status " + amh.getMessage("status.start"));
> -                            factory.createPanel(streamhandler, identifier, handle, x, y, url, atts);
> -
> -                            x += XDELTA;
> -                            y += YDELTA;
> -                            // make sure we don't go too far!
> -                            Dimension d = Toolkit.getDefaultToolkit().getScreenSize();
> -                            if ((x > d.width - 300) || (y > d.height - 300)) {
> -                                x = 0;
> -                                y = 2 * ydisp * YDELTA;
> -                                ydisp++;
> -                            }
> -                        }
> -                        atts = null;
> -                        isObjectTag = false;
> -                    }
> -                } else {
> -                    String nm = scanIdentifier(c, in);
> -                    if (nm.equalsIgnoreCase("param")) {
> -                        Hashtable<String, String> t = scanTag(c, in);
> -                        String att = t.get("name");
> -
> -                        if (att == null) {
> -                            statusMsgStream.println(requiresNameWarning);
> -                        } else {
> -                            String val = t.get("value");
> -                            if (val == null) {
> -                                statusMsgStream.println(requiresNameWarning);
> -                            } else {
> -                                PluginDebug.debug("PUT ", att, " = ", val);
> -                                atts.put(att.toLowerCase(), val);
> -                            }
> -                        }
> -                    } else if (nm.equalsIgnoreCase("applet")) {
> -                        atts = scanTag(c, in);
> -
> -                        // If there is a classid and no code tag present, transform it to code tag
> -                        if (atts.get("code") == null && atts.get("classid") != null
> -                                && !(atts.get("classid")).startsWith("clsid:")) {
> -                            atts.put("code", atts.get("classid"));
> -                        }
> -
> -                        // remove java: from code tag
> -                        if (atts.get("code") != null && (atts.get("code")).startsWith("java:")) {
> -                            atts.put("code", (atts.get("code")).substring(5));
> -                        }
> -
> -                        if (atts.get("code") == null && atts.get("object") == null) {
> -                            statusMsgStream.println(appletRequiresCodeWarning);
> -                            atts = null;
> -                        }
> -
> -                        if (atts.get("width") == null || !isInt(atts.get("width"))) {
> -                            atts.put("width", width);
> -                        }
> -
> -                        if (atts.get("height") == null || !isInt(atts.get("height"))) {
> -                            atts.put("height", height);
> -                        }
> -                    } else if (nm.equalsIgnoreCase("object")) {
> -                        isObjectTag = true;
> -
> -                        // Once code is set, additional nested objects are ignored
> -                        if (!objectTagAlreadyParsed) {
> -                            objectTagAlreadyParsed = true;
> -                            atts = scanTag(c, in);
> -                        }
> -
> -                        // If there is a classid and no code tag present, transform it to code tag
> -                        if (atts.get("code") == null && atts.get("classid") != null
> -                                && !(atts.get("classid")).startsWith("clsid:")) {
> -                            atts.put("code", atts.get("classid"));
> -                        }
> -
> -                        // remove java: from code tag
> -                        if (atts.get("code") != null && (atts.get("code")).startsWith("java:")) {
> -                            atts.put("code", (atts.get("code")).substring(5));
> -                        }
> -
> -                        // java_* aliases override older names:
> -                        //http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#in-ie
> -                        if (atts.get("java_code") != null) {
> -                            atts.put("code", (atts.get("java_code")));
> -                        }
> -
> -                        if (atts.containsKey("code")) {
> -                            objectTagAlreadyParsed = true;
> -                        }
> -
> -                        if (atts.get("java_codebase") != null) {
> -                            atts.put("codebase", (atts.get("java_codebase")));
> -                        }
> -
> -                        if (atts.get("java_archive") != null) {
> -                            atts.put("archive", (atts.get("java_archive")));
> -                        }
> -
> -                        if (atts.get("java_object") != null) {
> -                            atts.put("object", (atts.get("java_object")));
> -                        }
> -
> -                        if (atts.get("java_type") != null) {
> -                            atts.put("type", (atts.get("java_type")));
> -                        }
> -
> -                        if (atts.get("width") == null || !isInt(atts.get("width"))) {
> -                            atts.put("width", width);
> -                        }
> -
> -                        if (atts.get("height") == null || !isInt(atts.get("height"))) {
> -                            atts.put("height", height);
> -                        }
> -                    } else if (nm.equalsIgnoreCase("embed")) {
> -                        atts = scanTag(c, in);
> -
> -                        // If there is a classid and no code tag present, transform it to code tag
> -                        if (atts.get("code") == null && atts.get("classid") != null
> -                                && !(atts.get("classid")).startsWith("clsid:")) {
> -                            atts.put("code", atts.get("classid"));
> -                        }
> -
> -                        // remove java: from code tag
> -                        if (atts.get("code") != null && (atts.get("code")).startsWith("java:")) {
> -                            atts.put("code", (atts.get("code")).substring(5));
> -                        }
> -
> -                        // java_* aliases override older names:
> -                        //http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#in-nav
> -                        if (atts.get("java_code") != null) {
> -                            atts.put("code", (atts.get("java_code")));
> -                        }
> -
> -                        if (atts.get("java_codebase") != null) {
> -                            atts.put("codebase", (atts.get("java_codebase")));
> -                        }
> -
> -                        if (atts.get("java_archive") != null) {
> -                            atts.put("archive", (atts.get("java_archive")));
> -                        }
> -
> -                        if (atts.get("java_object") != null) {
> -                            atts.put("object", (atts.get("java_object")));
> -                        }
> -
> -                        if (atts.get("java_type") != null) {
> -                            atts.put("type", (atts.get("java_type")));
> -                        }
> -
> -                        if (atts.get("code") == null && atts.get("object") == null) {
> -                            statusMsgStream.println(embedRequiresCodeWarning);
> -                            atts = null;
> -                        }
> -
> -                        if (atts.get("width") == null || !isInt(atts.get("width"))) {
> -                            atts.put("width", width);
> -                        }
> -
> -                        if (atts.get("height") == null || !isInt(atts.get("height"))) {
> -                            atts.put("height", height);
> -                        }
> -
> -                    }
> -                }
> -            }
> -        }
> -        in.close();
> -    }
> -
> -    private static AppletMessageHandler amh = new AppletMessageHandler("appletviewer");


So nice removal :)
>
>       private static void checkConnect(URL url) {
>           SecurityManager security = System.getSecurityManager();
> diff --git a/tests/netx/unit/sun/applet/PluginAppletAttributesTest.java b/tests/netx/unit/sun/applet/PluginAppletAttributesTest.java
> new file mode 100644
> --- /dev/null
> +++ b/tests/netx/unit/sun/applet/PluginAppletAttributesTest.java
> @@ -0,0 +1,131 @@
> +package sun.applet;
> +
> +import static org.junit.Assert.assertEquals;
> +import static org.junit.Assert.assertTrue;
> +import static org.junit.Assert.assertFalse;
> +
> +import java.io.IOException;
> +import java.util.Hashtable;
> +import java.util.List;
> +
> +import org.junit.Test;
> +
> +public class PluginAppletAttributesTest {
> +
> +    @Test
> +    public void testIsInt() {
> +        assertFalse(PluginAppletAttributes.isInt("1.0"));
> +        assertFalse(PluginAppletAttributes.isInt("abc"));
> +        assertTrue(PluginAppletAttributes.isInt("1"));
> +    }
> +
> +    @Test
> +    public void testDecodeString() {
> +        assertEquals("< <> >",
> +                PluginAppletAttributes.decodeString("&lt; &lt;&gt; &gt;"));
> +        assertEquals("\" \" \" \"",
> +                PluginAppletAttributes.decodeString("&quot; &quot; &quot; &quot;"));
> +        assertEquals("& & &",
> +                PluginAppletAttributes.decodeString("&amp; &amp; &amp;"));
> +    }
> +
> +    @Test
> +    public void testExtractQuotedStrings() {
> +        String teststr1 = "\"1\" \"2\" \"3\" \"4\"";
> +        List<String> extracted1 = PluginAppletAttributes.extractQuotedStrings(teststr1);
> +
> +        assertEquals(extracted1.size(), 4);
> +
> +        assertEquals("1", extracted1.get(0));
> +        assertEquals("2", extracted1.get(1));
> +        assertEquals("3", extracted1.get(2));
> +        assertEquals("4", extracted1.get(3));
> +
> +        String teststr2 = "\"space\" \"after\" ";
> +
> +        List<String> extracted2 = PluginAppletAttributes.extractQuotedStrings(teststr2);
> +        assertEquals(2, extracted2.size());
> +
> +        assertEquals("space", extracted2.get(0));
> +        assertEquals("after", extracted2.get(1));
> +
> +    }
> +
> +    @Test
> +    public void testParseQuotedKeyValuePairs() {
> +        final String quotedPartString = "\"key\" \"val\" \"KEY&amp;\" \"&quot;\"";
> +        Hashtable<String, String> atts = PluginAppletAttributes.parseQuotedKeyValuePairs(quotedPartString);
> +
> +        assertEquals(atts.size(), 2);
> +
> +        assertEquals(atts.get("key"), "val");
> +        //ensure key is lowercased
> +        assertEquals(atts.get("key&"), "\"");
> +    }
> +
> +    @Test
> +    public void testAttributeParseJavaPrefix() {
> +        // java_* aliases override older names:
> +        //http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#in-nav
> +
> +        final String width = "1", height = "1";
> +        final String codeKeyVal = "\"code\" \"codeValue\" ";
> +
> +        Hashtable<String, String> atts;
> +
> +        atts = PluginAppletAttributes.parse(width, height,
> +                codeKeyVal + "\"java_code\" \"java_codeValue\" ");
> +        assertEquals("java_codeValue", atts.get("code"));
> +
> +        atts = PluginAppletAttributes.parse(width, height,
> +                codeKeyVal + "\"codebase\" \"codebaseValue\" \"java_codebase\" \"java_codebaseValue\" ");
> +        assertEquals("java_codebaseValue", atts.get("codebase"));
> +
> +        atts = PluginAppletAttributes.parse(width, height,
> +                codeKeyVal + "\"archive\" \"archiveValue\" \"java_archive\" \"java_archiveValue\" ");
> +        assertEquals("java_archiveValue", atts.get("archive"));
> +
> +        atts = PluginAppletAttributes.parse(width, height,
> +                codeKeyVal + "\"object\" \"objectValue\" \"java_object\" \"java_objectValue\" ");
> +        assertEquals("java_objectValue", atts.get("object"));
> +
> +        atts = PluginAppletAttributes.parse(width, height,
> +                codeKeyVal + "\"type\" \"typeValue\" \"java_type\" \"java_typeValue\" ");
> +        assertEquals("java_typeValue", atts.get("type"));
> +    }
> +
> +    @Test
> +    public void testAttributeParseWidthHeightAttributes() {
> +        final String width = "1", height = "1";
> +        final String codeKeyVal = "\"code\" \"codeValue\" ";
> +
> +        Hashtable<String, String> atts;
> +
> +        atts = PluginAppletAttributes.parse(width, height, codeKeyVal);
> +        assertEquals("1", atts.get("width"));
> +        assertEquals("1", atts.get("height"));
> +
> +        //Test that width height are defaulted to in case of not-a-number attributes:
> +        atts = PluginAppletAttributes.parse(width, height, codeKeyVal + " \"width\" \"NAN\" \"height\" \"NAN\" ");
> +        assertEquals("1", atts.get("width"));
> +        assertEquals("1", atts.get("height"));
> +    }
> +
> +    @Test
> +    public void testAttributeParseCodeAttribute() {
> +        final String width = "1", height = "1";
> +        Hashtable<String, String> atts;
> +
> +        atts = PluginAppletAttributes.parse(width, height, "\"classid\" \"classidValue\" ");
> +        assertEquals("classidValue", atts.get("code"));
> +
> +        atts = PluginAppletAttributes.parse(width, height, "\"code\" \"java:codeValue\" ");
> +        assertEquals("codeValue", atts.get("code"));
> +
> +        atts = PluginAppletAttributes.parse(width, height, "\"classid\" \"clsid:classidValue\" ");
> +        assertEquals(null, atts); //NB: atts == null if theres no object or code attribute
> +
> +        atts = PluginAppletAttributes.parse(width, height, "\"object\" \"objectValue\" ");
> +        assertEquals("objectValue", atts.get("object"));
> +    }
> +}


I do not want to be pedantic, but how deep is coverage of those?
And probably tehre will come need to test possible new ParsedAppletTAg class.
>

Thanx for valuable patch!

J.




More information about the distro-pkg-dev mailing list