[rfc][icedtea-web] Fix accessing Java array elements from Javascript

[Jie Kang]

----- Original Message -----
> On 01/27/2015 04:52 PM, Jie Kang wrote:
> >
> >
> > ----- Original Message -----
> >> On 01/26/2015 04:17 PM, Jie Kang wrote:
> >>> Hello,
> >>>
> >>> Marcin Cieslak has taken the time to create a fix for a long-standing
> >>> problem in Icedtea-Web where attempting to set/get Java array elements
> >>> from Javascript fails. [1]
> >>>
> >>> [1] http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1298
> >>>
> >>>
> >>> The reproducers relevant to this fix are:
> >>>
> >>> simple/JToJSString
> >>>
> >>> These reproducers tests on F21 with OpenJDK 7 and HEAD patched are now
> >>> passing for me. These aren't in 1.5.x
> >>>
> >>>
> >>> simple/JSToJSet
> >>>
> >>> These reproducers are still failing. On further investigation, the
> >>> reproducer itself is not working correctly. A fix is needed here and work
> >>> is in progress.
> >>>
> >>> I have created a simple reproducer that mimics
> >>> AppletJSToJSet_intArrayElement_Test and it passes. This is attached as:
> >>> itw-js-int-array-test-1.patch. This reproducer passes on both 1.5 and
> >>> HEAD.
> >>>
> >>>
> >>> I think a backport to 1.5.x would also be great. The two patches for HEAD
> >>> and 1.5 are attached as: itw-js-array-HEAD-1.patch and
> >>> itw-js-array-1.5-1.patch.
> >>>
> >>> Thoughts?
> >>>
> >>>
> >>> Thank you again Marcin!
> >>>
> >>>
> >>
> >> The code of fix is ok AFAICS. Thank you both!
> >>
> >> For fixes: Please include  NEWS entry and add Marcin to authors.
> >>
> >> For thests:  SimpleJNLP.html  and SimpleJNLP.jnlp have missing license
> >> headers
> >>                SimpleJNLPTest.java - can it be reworked to work with
> >>                default
> >>                ClosingListeners?
> >
> > Hello,
> >
> > Just a quick note here:
> >
> > Sorry I didn't explain fully.
> >
> > The SimpleJNLP tests are just to show that the patch works; I didn't intend
> > them for review. I included it in the e-mail because the original test in
> > HEAD, JSToJSet, is actually broken, so they don't show that the patch
> > worked. I will need to fix them.
> >
> > The SimpleJNLP test is just a small copy of JSToJSet test that is fixed so
> > I don't think SimpleJNLP needs to go for review/push. I will fix the
> > JSToJSet test and send patch for that in separate e-mail.
> 
> Ah. sure. And why not to include them? It seems to be relevant test worthy to
> be added to
> reproducers suite.


Uhm, the SimpleJNLP test is a copy of a test inside JSToJSet (in HEAD). So the test is already in the reproducers suite (but currently broken).

> 
> >
> > Is that okay with you?
> >
> Still please post updated versions of head and 1.5 patch.

Yes of course.

> >>
> >>
> >> Please turn one more round of review before push.
> >
> > Okay, will do NEWS + Author and send for review soon.
> >
> >
> > Regards,
> >
> >>
> >> Thank you both!
> >>     J.
> >>
> >>
> >>
> >>
> >>
> >>
> >
> 
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern