[rfc][icedtea-web] modifications to JSToJSet reproducer for Liveconnect

[Jana Fabrikova]

Hello Adam,

thank you for your comments. I do not know precisely what causes the 
problems while setting array elements from JS to Java.

I have changed the JSToJSet testcase with added @Bug annotation (bug 
number PR1298 filed, since the other bugs mentioning "applet" or "array" 
seemed different to me). The enum TestType has also been included,please 
find the modified patch attached,

Jana

On 02/05/2013 09:11 PM, Adam Domurad wrote:
> On 01/21/2013 04:35 AM, Jana Fabrikova wrote:
>> Hello,
>>
>> i am sending a patch that modifies the JSToJSet reproducer (adding two
>> KnownToFail annotations and adding one testcase - setting a variable
>> of the JSObject type),
>>
>> comments on the attached patch are very welcome,
>> Jana
>>
>> 2013-01-21 Jana Fabrikova <jfabriko at redhat.com>
>>
>> * /tests/reproducers/simple/JSToJSet/testcases/JSToJSetTest.java:
>> adding 1 testcase setting applets variable of type JSObject from JS,
>> adding KnownToFail anotation to (AppletJSToJSet_intArrayElement_Test)
>> and (AppletJSToJSet_DoubleArrayElement_Test) methods
>>
>> * /tests/reproducers/simple/JSToJSet/resources/JSToJava_Set.js:
>> adding the JSObject case to (doSetTests) function
>>
>> * /tests/reproducers/simple/JSToJSet/srcs/JSToJSet.java:
>> adding the JSObject variable to the applet and modifying
>> (printNewValueAndFinish) method in order to output new values of
>> JSObject variable
>
> (Just a note, make sure the ChangeLog is in proper ChangeLog format --
> ie explanation lines dont exceed 80 lines, and must start with a tab)
>
>> diff --git
>> a/tests/reproducers/simple/JSToJSet/resources/JSToJava_Set.js
>> b/tests/reproducers/simple/JSToJSet/resources/JSToJava_Set.js
>> --- a/tests/reproducers/simple/JSToJSet/resources/JSToJava_Set.js
>> +++ b/tests/reproducers/simple/JSToJSet/resources/JSToJava_Set.js
>> @@ -1,3 +1,9 @@
>> +//dummy javascript class whose instance is passed as JSObject parameter:
>> +function JSCar(mph,color){
>> + this.mph = mph;
>> + this.color = color;
>> +}
>> +
>> function doSetTests( ){
>>
>> var urlArgs = document.URL.split("?");
>> @@ -18,12 +24,16 @@
>> if( field === "_specialString"){
>> value = "��〒£$ǣ€��";
>> }
>> +
>> + if( field === "_JSObject"){
>> + value = new JSCar(100,"red");
>> + }
>> +
>> }else if(value.indexOf('[') != -1){
>>
>> var elem = value.substring(1);
>> value = new Array();
>> eval('value[0] = elem');
>> -
>> }
>>
>> eval('applet.' + field + '= value');
>> diff --git a/tests/reproducers/simple/JSToJSet/srcs/JSToJSet.java
>> b/tests/reproducers/simple/JSToJSet/srcs/JSToJSet.java
>> --- a/tests/reproducers/simple/JSToJSet/srcs/JSToJSet.java
>> +++ b/tests/reproducers/simple/JSToJSet/srcs/JSToJSet.java
>> @@ -1,7 +1,7 @@
>> -import java.applet.*;
>> -import java.awt.*;
>> +import java.applet.Applet;
>> import java.lang.reflect.Array;
>> import java.lang.reflect.Field;
>> +import netscape.javascript.JSObject;
>>
>> public class JSToJSet extends Applet {
>>
>> @@ -27,6 +27,7 @@
>> public Double[] _DoubleArray2;
>> public char[] _charArray = new char[1];
>> public Character[] _CharacterArray = new Character[1];
>> + public JSObject _JSObject;
>>
>> public void init() {
>> String initStr = "JSToJSet applet initialized.";
>> @@ -36,7 +37,12 @@
>> public void printNewValueAndFinish(String fieldname) throws Exception {
>> Field field = getClass().getDeclaredField(fieldname);
>> Object value = field.get(this);
>> - if (value != null && value.getClass().isArray()) {
>> +
>> + if( fieldname.equals("_JSObject") ){
>> + Integer mph = (Integer)_JSObject.getMember("mph");
>> + String color = (String)_JSObject.getMember("color");
>> + System.out.println("New value is: "+mph+", "+color);
>> + }else if (value != null && value.getClass().isArray()) {
>> System.out.println("New array value is: " + Array.get(value, 0));
>> } else {
>> System.out.println("New value is: " + value);
>> diff --git
>> a/tests/reproducers/simple/JSToJSet/testcases/JSToJSetTest.java
>> b/tests/reproducers/simple/JSToJSet/testcases/JSToJSetTest.java
>> --- a/tests/reproducers/simple/JSToJSet/testcases/JSToJSetTest.java
>> +++ b/tests/reproducers/simple/JSToJSet/testcases/JSToJSetTest.java
>> @@ -40,6 +40,7 @@
>> import net.sourceforge.jnlp.browsertesting.BrowserTest;
>> import net.sourceforge.jnlp.browsertesting.Browsers;
>> import net.sourceforge.jnlp.closinglisteners.CountingClosingListener;
>> +import net.sourceforge.jnlp.annotations.KnownToFail;
>> import net.sourceforge.jnlp.annotations.NeedsDisplay;
>> import net.sourceforge.jnlp.annotations.TestInBrowsers;
>> import org.junit.Assert;
>> @@ -71,7 +72,7 @@
>>
>> // Assert that the values set by JavaScript are ok
>> Assert.assertTrue("JSToJSet: the output should include:
>> "+expectedStdout+", but it didnt.",
>> - pr.stdout.contains(expectedStdout));
>> + pr.stdout.contains(expectedStdout));
>>
>> }
>>
>> @@ -83,23 +84,23 @@
>> }
>>
>> private void jsToJavaSetSpecialTest(String fieldStr, String valueStr,
>> int testType) throws Exception {
>> - String strURL = "/JSToJSet.html?";
>> - String expectedStdout = "";
>> - switch( testType ){
>> + String strURL = "/JSToJSet.html?";
>> + String expectedStdout = "";
>> + switch( testType ){
>> case 0://array element
>> - strURL += fieldStr + ";" + valueStr;
>> - expectedStdout = "New array value is: "+valueStr;
>> + strURL += fieldStr + ";" + valueStr;
>> + expectedStdout = "New array value is: "+valueStr;
>> break;
>> case 1://whole array, set 1st element
>> - strURL += fieldStr + ";[" + valueStr;
>> - expectedStdout = "New array value is: "+valueStr;
>> - break;
>> + strURL += fieldStr + ";[" + valueStr;
>> + expectedStdout = "New array value is: "+valueStr;
>> + break;
>> case 2://char et al - to be set at JS side
>> - strURL += fieldStr + ";JavaScript";
>> - expectedStdout = "New value is: "+valueStr;
>> + strURL += fieldStr + ";JavaScript";
>> + expectedStdout = "New value is: "+valueStr;
>> break;
>> default:
>> - break;
>> + break;
>> }
>
> Can you change this 0,1,2 to an enum ?
> eg
> enum TestType {
> ARRAY_ELEMENT,
> WHOLE_ARRAY,
> NORMAL_VALUE
> }
> Or something, and use those in the switch statement instead.
>
>>
>> ProcessResult pr = server.executeBrowser(strURL, new
>> CountingClosingListenerImpl(), new CountingClosingListenerImpl());
>> @@ -158,6 +159,7 @@
>> @Test
>> @TestInBrowsers(testIn = { Browsers.all })
>> @NeedsDisplay
>> + @KnownToFail
>> public void AppletJSToJSet_intArrayElement_Test() throws Exception {
>> jsToJavaSetSpecialTest("_intArray[0]", "1", 0);
>> }
>> @@ -235,6 +237,7 @@
>> @Test
>> @TestInBrowsers(testIn = { Browsers.all })
>> @NeedsDisplay
>> + @KnownToFail
>
> Interesting, would you mind filing a bug report at
> http://icedtea.classpath.org/bugzilla if one doesn't exist yet ? Thanks!
> I looked into it a bit and it indeed seems to be a bug, but do you have
> any insight into what's causing it ?
>
>> public void AppletJSToJSet_DoubleArrayElement_Test() throws Exception {
>> jsToJavaSetSpecialTest("_DoubleArray[0]", "1.1", 0);
>> }
>> @@ -246,4 +249,10 @@
>> jsToJavaSetSpecialTest("_DoubleArray2", "0.1", 1);
>> }
>>
>> + @Test
>> + @TestInBrowsers(testIn = { Browsers.all })
>> + @NeedsDisplay
>> + public void AppletJSToJSet_JSObject_Test() throws Exception {
>> + jsToJavaSetSpecialTest("_JSObject", "100, red", 2);
>> + }
>> }
>
> Looks good otherwise!
> -Adam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: JSToJSet_enum_bugannot.patch
Type: text/x-patch
Size: 7848 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130206/39ae8e7f/JSToJSet_enum_bugannot.patch