Hi Alex,

        I have updated the webrev with changes for deserialization,
        http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.02/

        Please review.
        Verified that serialization & de-serialization of text field object 
works fine.
        Also updated the test in this webrev to test de-serialization.

Many Thanks,
Ambarish

-----Original Message-----
From: Alexander Scherbatiy 
Sent: Thursday, October 29, 2015 9:59 PM
To: Ambarish Rapte
Cc: Prasanta Sadhukhan; Semyon Sadetsky; awt-dev@openjdk.java.net; Sergey 
Bylokhov
Subject: Re: Review Request for 8055197: TextField deletes multiline strings

On 10/29/2015 5:31 PM, Ambarish Rapte wrote:
> Hi Alexandr,
>
>       Please review the updated webrev, with changes only in TextField.java.
>
>       Webrev: 
> http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.01/
>
>
>       replaceEOL() is declared static, as a class member method cannot be 
> called before call to super from Constructor.
>       Also made changes, as per Sergey's comment for, 
> System.lineSeparator()

      Thank you.

      Could you also check the TextField deserialization? It always should be 
kept in mind that fields in a deserialized object need to passe the same checks 
which are applied in constructors and set methods.

    Thanks,
    Alexandr.
>
> Many Thanks,
> Ambarish
>
> -----Original Message-----
> From: Alexander Scherbatiy
> Sent: Wednesday, October 28, 2015 8:48 PM
> To: Ambarish Rapte
> Cc: awt-dev@openjdk.java.net; Sergey Bylokhov
> Subject: Re: Review Request for 8055197: TextField deletes multiline 
> strings
>
> On 10/26/2015 11:21 PM, Ambarish Rapte wrote:
>> Hi Alexandr,
>>      Yes, This is windows specific issue.
>>      Initially there was a native side fix, Please refer below webrev for 
>> the earlier changes.
>>      http://cr.openjdk.java.net/~psadhukhan/ambarish/8055197/webrev.00/
>>
>>      But there was an issue pointed for this fix, and after discussion with 
>> Phil,
>>      We arrived at this new solution, of replacing on java side.
>>      http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.00/
>>
>>
>>
>>      A major review comment for this previous fix was,
>>
>>      Sergey:
>>      Note that the fix replace the eol char on the peer level. So there will
>>         be a difference in behavior of setText/GetText when the peer 
>> exists or not
>         I see the point now.
>
>        Is it possible to add the 'String replaceEOL(String text)' method to 
> the TextField? In this case TextField constructors can remove EOL from the 
> given text and pass it to the super class. It could avoid the TextComponent 
> class changing.
>
>      Thanks,
>      Alexandr.
>
>>
>>      However both the fix have no side effect, Have executed jtreg on almost 
>> all tests which use or test TextField.
>>
>>      There is confusion for most correct way to patch this. Please guide
>>
>> Thanks
>> Ambarish
>>
>>
>> -----Original Message-----
>> From: Alexander Scherbatiy
>> Sent: Wednesday, October 21, 2015 7:29 PM
>> To: Ambarish Rapte
>> Cc: awt-dev@openjdk.java.net; Sergey Bylokhov
>> Subject: Re: Review Request for 8055197: TextField deletes multiline 
>> strings
>>
>>
>>
>>      Is this is Windows specific issue only? If so, it is better to replace 
>> EOL on window text peer side or may be even better to do it on the native 
>> side before setting a text to the single-line RichEdit. May be there are 
>> methods that can remove EOL from a string exactly in the same way as it is 
>> done by Edit control.
>>
>>     Thanks,
>>     Alexandr.
>>
>> On 10/19/2015 9:31 PM, Ambarish Rapte wrote:
>>> Hi Alexander,
>>>
>>>     In single line mode , there is no rich edit control style to handle EOL.
>>>     Referenced documentation:
>>> https://msdn.microsoft.com/en-us/library/windows/desktop/bb787605(v=
>>> v
>>> s
>>> .85).aspx
>>>
>>> Many Thanks
>>> Ambarish
>>>
>>> -----Original Message-----
>>> From: Alexander Scherbatiy
>>> Sent: Friday, October 16, 2015 4:45 PM
>>> To: Ambarish Rapte
>>> Cc: awt-dev@openjdk.java.net; Sergey Bylokhov
>>> Subject: Re: Review Request for 8055197: TextField deletes multiline 
>>> strings
>>>
>>>
>>>       The TextField is based on Rich Edit control on Windows. Does the Rich 
>>> Edit control have properties to properly handle multiline strings in 
>>> single-line mode?
>>>
>>>       Thanks,
>>>       Alexandr.
>>>
>>> On 10/14/2015 7:44 PM, Ambarish Rapte wrote:
>>>> Dear All,
>>>>
>>>>         Please review the patch for jdk9.
>>>>         Bug: https://bugs.openjdk.java.net/browse/JDK-8055197
>>>>         Webrev:
>>>> http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.00/
>>>>
>>>>
>>>> Issue:
>>>>         - If text containing new line character is set to TextField, Text 
>>>> after new line character was terminated.
>>>>         - Issue occurs only on windows.
>>>>
>>>> Cause:
>>>>         - For windows new line character is   ā€˜\r\nā€™.
>>>>         - For windows code this new line character was not replaced by 
>>>> space character.
>>>>         - Other platforms replace the EOL character by space character.
>>>>
>>>> Fix:
>>>>         - Added code to TextComponent.java to remove EOL on java side 
>>>> before passing to peer.
>>>>           => Added a private method replaceEOL() , which replaces EOL by 
>>>> space.
>>>>           => removeEOL will be called by newly added TextComponent 
>>>> construcotr and setText()
>>>>
>>>>         - The text variable on in TextComponent.java & on string displayed 
>>>> on native side will be same.
>>>>
>>>>
>>>>
>>>> Many Thanks,
>>>> Ambarish

Reply via email to