[rfc][icedtea-web] Remove need for HTML tag scanner in PluginAppletViewer
Adam Domurad
adomurad at redhat.com
Tue Oct 30 13:06:19 PDT 2012
On 10/26/2012 09:13 AM, Jiri Vanek wrote:
> 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, " ");
>> + encoded += " ";
>> else if (to_encode[i] == '\n')
>> - encoded = strcat(encoded, " ");
>> + encoded += " ";
>> else if (to_encode[i] == '>')
>> - encoded = strcat(encoded, ">");
>> + encoded += ">";
>> else if (to_encode[i] == '<')
>> - encoded = strcat(encoded, "<");
>> + encoded += "<";
>> else if (to_encode[i] == '&')
>> - encoded = strcat(encoded, "&");
>> + encoded += "&";
>> else if (to_encode[i] == '"')
>> - encoded = strcat(encoded, """);
>> + encoded += """;
>> 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(">", ">");
>> + toDecode = toDecode.replace("<", "<");
>> + toDecode = toDecode.replace(" ", "\n");
>> + toDecode = toDecode.replace(" ", "\r");
>> + toDecode = toDecode.replace(""", "\"");
>> + toDecode = toDecode.replace("&", "&");
>> +
>> + 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(">", ">");
>> - toDecode = toDecode.replace("<", "<");
>> - toDecode = toDecode.replace("&", "&");
>> - toDecode = toDecode.replace(" ", "\n");
>> - toDecode = toDecode.replace(" ", "\r");
>> - toDecode = toDecode.replace(""", "\"");
>> -
>> - 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("< <>
>> >"));
>> + assertEquals("\" \" \" \"",
>> + PluginAppletAttributes.decodeString("" "
>> " ""));
>> + assertEquals("& & &",
>> + PluginAppletAttributes.decodeString("& &
>> &"));
>> + }
>> +
>> + @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&\" \""\"";
>> + 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.
>
Thanks for the comments, I hope to have addressed them. Attached is
iteration 2, let me know what you think of the direction it is going.
This patch introduces net.sourceforge.jnlp.PluginParameters as well as
sun.applet.PluginParameterParser. I have removed the use of static
method as suggested.
The decoded was replaced by a simple escaping.
The wrapped PluginParameter's class was used where I thought was
logical, please comment especially on the placement of this class where
Hashtable was previously used.
Thanks for the review & hints,
- Adam
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nuke-parser2.patch
Type: text/x-patch
Size: 73227 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121030/0cdaa1df/nuke-parser2.patch
More information about the distro-pkg-dev
mailing list