<AWT Dev> Review Request for JDK-8040322 : TextArea.replaceRange() and insert() are broken with setText(null)

[Alexander Zvegintsev]

Looks fine to me too.

Thanks,

Alexander.

On 09/28/2015 02:11 PM, Ambarish Rapte wrote:
> Hi Sergey,
> 	Thanks for the review.
> 	Executed regression tests for TextArea, TextField & also other tests which use any of the TextArea, TextField or TextComponent.
> 	=> There are NO failures due this patch change.
>
> Dear All,
> 	One more review is required for this patch.
> 	Please review.
> 		
>
> Many Thanks,
> Ambarish Rapte
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Wednesday, September 23, 2015 6:36 PM
> To: Ambarish Rapte; Alexander Scherbatiy; awt-dev at openjdk.java.net
> Subject: Re: Review Request for JDK-8040322 : TextArea.replaceRange() and insert() are broken with setText(null)
>
> The fix looks fine to me. Thanks.
> You can also run related regression tests from java/awt to check that there are no regressions.
>
> On 21.09.15 12:38, Ambarish Rapte wrote:
>> Hi,
>>
>>                   The previous mail for review of this issue
>> JDK-8040322,
>>
>>                   is filtered with another issue & issues are merged.
>>
>>                   So I am writing a new mail to continue the review on
>> separate thread.
>>
>> Dear Sergey,
>>
>>                   Thanks for the review. Updated the patch according to
>> your review comments.
>>                   Please take a look.
>>
>>                   Please review this patch at
>>
>>                   Bug: https://bugs.openjdk.java.net/browse/JDK-8040322
>>
>>                   Webrev :
>> http://cr.openjdk.java.net/~psadhukhan/ambarish/8040322/webrev.01/
>>
>> Below is history from previous mail discussion,
>>
>> Hi, Ambarish.
>>
>> A few comments.
>>
>>     - The fix changed the order of methods calls. Note that if the user
>> overrides the "appendText" then it is called after an "append".
>>
>>     - It seems that before the fix we always tried to use the empty
>> string for a null text(we replace null to "" in the constructor and
>> setText), the new code should maintain the same assumption.
>>
>> On 18.09.15 14:36, Ambarish Rapte wrote:
>>
>>   > Hi,
>>
>>   >
>>
>>   >                  Please review the following fix for jdk9.
>>
>>   >
>>
>>   > Bug:https://bugs.openjdk.java.net/browse/JDK-8040322
>>
>>   > Webrev:http://cr.openjdk.java.net/~psadhukhan/ambarish/8040322/webrev.
>>
>>   > 00/
>>
>>   >
>>
>>   > Issue:
>>
>>   > /            TextArea.setText(null)/   does not set TextArea text to
>>
>>   > /null/ when called after
>>
>>   > /TextArea.replaceRange(), TextArea.insert(),  TextArea.append()./
>>
>>   >
>>
>>   > Cause:
>>
>>   >                  Variable "/text/"  was not correctly updated in
>>
>>   >
>>
>>   > /TextArea.replaceRange(), TextArea.insert(),  TextArea.append()/
>> in
>>
>>   > awt / TextArea.java
>>
>>   >
>>
>>   > Fix:
>>
>>   >                  Update the variable "text" correctly with the calls
>>
>>   > to
>>
>>   >
>>
>>   > /TextArea.replaceRange(), TextArea.insert(),  TextArea.append()/
>>
>>   >
>>
>>   > In file awt / TextArea.java
>>
>>   >
>>
>>   > Many Thanks,
>>
>>   > Ambarish Rapte
>>
>>   >
>>
>> --
>>
>> Best regards, Sergey.
>>
>> Many Thanks,
>> Ambarish Rapte
>>
>
> --
> Best regards, Sergey.