<AWT Dev> Review Request for 8055197: TextField deletes multiline strings

[Ambarish Rapte]

Dear Alex,

	Please take a look at updated webrev,
	http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.03/


	Updated the test code for testing de-serialization.


Many Thanks
Ambarish

-----Original Message-----
From: Alexander Scherbatiy 
Sent: Friday, November 06, 2015 5:36 PM
To: Ambarish Rapte
Cc: awt-dev at openjdk.java.net
Subject: Re: <AWT Dev> Review Request for 8055197: TextField deletes multiline strings

On 11/5/2015 2:34 PM, Alexander Scherbatiy wrote:
> On 11/5/2015 1:33 PM, Ambarish Rapte wrote:
>> 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.

     There is one thing to note that readObject() in the fix first use
s.defaultReadObject() that can read a value with new lines into text field so there would be a small time when the TextField has an inconsistence state. It is recommended to read all fields by
fields.get(fieldName) to avoid this:

     ObjectInputStream.GetField fields = in.readFields();
     String name = (String) fields.get("name", DEFAULT);

   Thanks,
   Alexandr.

> It is true if a serialized file was created by TextField 
> serialization. The text field does not contain new lines in this case 
> because they were removed by constructor or set method.
>       The question was about serialized files which were generated 
> manually or changed after a serialization. In this case the text field 
> can be rewritten with a value which contains new lines.
>
>    Thanks,
>    Alexandr.
>
>>
>> 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 at 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 at 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 at 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 at 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
>