<AWT Dev> Review Request for 8055197: TextField deletes multiline strings
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Wed Nov 25 12:56:54 UTC 2015
Then this version looks fine.
On 23.11.15 8:45, Ambarish Rapte wrote:
> Hi Sergey,
>
> ObjectInputStream.GetField fields = in.readFields(); => **Provides only current class fields & does not provide parent class fields.
>
> Below change in TextField::readObject(),
>
> //s.defaultReadObject();
> ObjectInputStream.GetField fields = s.readFields();
> String tx = (String) fields.get("text", "");
>
> causes java.lang.IllegalArgumentException,
> [Doc: http://docs.oracle.com/javase/8/docs/platform/serialization/spec/input.html#a4936 Paragraph 3, 2nd statement:-
> - An IllegalArgumentException is thrown if the requested field is not a serializable field of the CURRENT class]
>
>
> **** So above change cannot be done in TextField::readObject(), as "text" is member of TextComponent.
>
>
>
> To use ObjectInputStream.GetField fields = s.readFields(); ,the fix would be as below,
>
> Change in TextCompoenent::readObject(), as
>
> if (this instanceof TextField) {
> ObjectInputStream.GetField fields = s.readFields();
> String tx = (String) fields.get("text", "");
> text = replaceEOL(tx); //**** Add private replaceEOL in TextComponent.java as well
> } else {
> s.defaultReadObject();
> }
>
> => This approach,
> 1. Needs a change in TextComponent.java
> 2. Will create dependency of parent class on its own child. As this change is only for TextField & not for TextArea.
> 3. replaceEOL() needs to be duplicated in TextComponent & TextField, as replaceEOL should not be public.
>
>
> So we decided to go ahead with the current fix under review.
> http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.03/
>
>
> Many Thanks,
> Ambarish
>
>
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Monday, November 23, 2015 1:00 AM
> To: Alexander Scherbatiy; Ambarish Rapte
> Cc: awt-dev at openjdk.java.net
> Subject: Re: <AWT Dev> Review Request for 8055197: TextField deletes multiline strings
>
> Hello.
> On 06.11.15 15:05, Alexander Scherbatiy wrote:
>> 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);
>
> What about this proposal? Probably I missed it and it was discussed somewhere?
>
>
>>
>> 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
>>>
>>
>
>
--
Best regards, Sergey.
More information about the awt-dev
mailing list