<AWT Dev> Review Request for 8055197: TextField deletes multiline strings
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Fri Nov 6 12:05:45 UTC 2015
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