<AWT Dev> Review Request for 8055197: TextField deletes multiline strings
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Fri Nov 20 09:38:08 UTC 2015
The fix looks good to me.
Thanks,
Alexandr.
On 11/19/2015 11:43 AM, Ambarish Rapte wrote:
> 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
More information about the awt-dev
mailing list