<AWT Dev> Review request for 8060137: Removing Text from TextField / TextArea is not possible after typing

Semyon Sadetsky semyon.sadetsky at oracle.com
Fri Dec 4 08:08:58 UTC 2015


Hi Ambarish,

Thanks.
The fix looks good.

--Semyon

On 12/4/2015 10:55 AM, Ambarish Rapte wrote:
>
> Hi Semyon,
>
> peer.getText() does not return null,
>
> In rest code also, there are no null checks on  returned string of 
> peer.getText(),   for example,  inside TextComponent::getText().
>
> So, peer.getText(), would either return “”   or   a valid string.
>
> The test case also covers this case, that when there is no string 
>   TextComponent::getText()  returns empty string.
>
> Thanks,
>
> Ambarish
>
> *From:*Semyon Sadetsky
> *Sent:* Thursday, December 03, 2015 8:01 PM
> *To:* Ambarish Rapte; Prasanta Sadhukhan; awt-dev at openjdk.java.net
> *Subject:* Re: <AWT Dev> Review request for 8060137: Removing Text 
> from TextField / TextArea is not possible after typing
>
> Hi Ambarish,
>
> If peer.getText() returns null the new version will invoke 
> peer.setText(t) even if the t is empty or null. The original logic 
> skipped setText() in such scenario.
>
> --Semyon
>
> On 12/3/2015 11:51 AM, Ambarish Rapte wrote:
>
>     Hi,
>
>     Gentle reminder for review.
>
>     Regards,
>
>     Ambarish
>
>     *From:*Ambarish Rapte
>     *Sent:* Thursday, November 26, 2015 6:18 AM
>     *To:* Semyon Sadetsky; Prasanta Sadhukhan;
>     awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
>     *Subject:* Re: <AWT Dev> Review request for 8060137: Removing Text
>     from TextField / TextArea is not possible after typing
>
>     Hi Semyon,
>
>     Please review the updated webrev,
>
>     http://cr.openjdk.java.net/~arapte/8060137/webrev.03/
>     <http://cr.openjdk.java.net/%7Earapte/8060137/webrev.03/>
>
>     Changes:
>
>     1.Change as per the previous review comment.
>
>     2.Earlier setText() avoided peer.setText(), if new and previous
>     strings were “*null” *or*“empty”*.
>     => I have changed this to,  avoid peer.setText(), if the new and
>     previous strings are *“same”*.
>
>     Please review this change,
>
>     Many Thanks,
>
>     Ambarish
>
>     *From:*Semyon Sadetsky
>     *Sent:* Tuesday, November 24, 2015 12:53 PM
>     *To:* Ambarish Rapte; Prasanta Sadhukhan; awt-dev at openjdk.java.net
>     <mailto:awt-dev at openjdk.java.net>
>     *Subject:* Re: Review request for 8060137: Removing Text from
>     TextField / TextArea is not possible after typing
>
>     Hi Ambarish,
>        that is the original logic. I think it's better to leave it as
>     it is, otherwise we may receive a regression.
>
>     --Semyon
>
>     On 11/23/2015 2:26 PM, Ambarish Rapte wrote:
>
>         Hi Semyon,
>
>         The current code sets the TextComponent.java::text field, even
>         if peer is null.
>
>         So when peer is null, Java and peer side text will not be same.
>
>         Is this behavior fine & Expected ?
>
>         Thanks,
>
>         Ambarish
>
>         *From:*Semyon Sadetsky
>         *Sent:* Friday, November 20, 2015 6:22 PM
>         *To:* Ambarish Rapte; Prasanta Sadhukhan;
>         awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
>         *Subject:* Re: Review request for 8060137: Removing Text from
>         TextField / TextArea is not possible after typing
>
>         Hi Ambarish,
>
>         Didn't notice that was your fix, sorry...
>
>         One small issue: text = (t != null) ? t : ""; should be set
>         even if peer doesn't exist.
>
>         --Semyon
>
>         On 11/20/2015 2:49 PM, Ambarish Rapte wrote:
>
>             Hi ,
>
>             Updating the patch to use /(peer != null)  { } / instead
>             of /(peer == null) retrun;/
>
>             Please take a look
>
>             /http://cr.openjdk.java.net/~arapte/8060137/webrev.02/
>             <http://cr.openjdk.java.net/%7Earapte/8060137/webrev.02/>/
>
>             //
>
>             //
>
>             Many Thanks,
>
>             Ambarish
>
>             //
>
>             *From:*Ambarish Rapte
>             *Sent:* Thursday, November 19, 2015 9:19 PM
>             *To:* Semyon Sadetsky; Prasanta Sadhukhan;
>             awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
>             *Subject:* RE: Review request for 8060137: Removing Text
>             from TextField / TextArea is not possible after typing
>
>             Hi Semyon,
>
>             Please review the updated patch as per the review comments,
>
>             http://cr.openjdk.java.net/~arapte/8060137/webrev.01/
>             <http://cr.openjdk.java.net/%7Earapte/8060137/webrev.01/>
>
>             Many Thanks
>
>             Ambarish
>
>             *From:*Semyon Sadetsky
>             *Sent:* Thursday, November 19, 2015 2:54 PM
>             *To:* Ambarish Rapte; Prasanta Sadhukhan;
>             awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>
>             *Subject:* Re: Review request for 8060137: Removing Text
>             from TextField / TextArea is not possible after typing
>
>             Hi Prasanta,
>
>             Could you rework the fix a bit?
>             When peer != null is false there is no need to continue
>             the method execution. And then second peer is null test is
>             not needed.
>
>             --Semyon
>
>             On 11/16/2015 1:24 PM, Ambarish Rapte wrote:
>
>                 Dear All,
>
>                                 Please review the fix for JDK9,
>
>                                 Bug:
>                 https://bugs.openjdk.java.net/browse/JDK-8060137
>
>                                 Webrev:
>                 http://cr.openjdk.java.net/~arapte/8060137/webrev.00/
>                 <http://cr.openjdk.java.net/%7Earapte/8060137/webrev.00/>
>
>                 Issue:
>
>                 1.Type any character in /TextArea/ or /TextField/
>
>                 2.Call /setText(null)/
>
>                 ðThe text in /TextArea/ or /TextField/ does not get
>                 set to null.
>
>                 Cause:
>
>                 /TextComponent::setText()/, verifies
>                 /TextComponent::text/ variable for null value in
>                 /TextComponent::setText()/.
>
>                                 But text is a java variable which may
>                 not have latest value of actual at peer side.
>
>                 Fix:
>
>                                 Fetch the latest value from
>                 /peer.getText(),/ before validating for null value.
>
>                                 Also updated tests for /TextArea/ &
>                 /TextField/ with the patch.
>
>                 Many Thanks,
>
>                 Ambarish
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20151204/a04aeace/attachment-0001.html>


More information about the awt-dev mailing list