Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin [v2]

2021-07-23 Thread Marius Hanl
On Wed, 7 Jul 2021 09:48:20 GMT, Jeanette Winzenburg wrote: >> The issue is about memory leaks and side-effects (like NPEs) when switching >> skins. >> >> Details (copied from issue for convenience): >> >> memory leak in TextInputControlBehavior: >> - listener accidentally added twice

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin [v2]

2021-07-12 Thread Jeanette Winzenburg
On Mon, 28 Jun 2021 13:44:16 GMT, Marius Hanl wrote: >> Jeanette Winzenburg has updated the pull request incrementally with one >> additional commit since the last revision: >> >> addressed review issues > > Just a formal review, I left some comments inline @Maran23 did resolve all we

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin [v2]

2021-07-08 Thread Ambarish Rapte
On Wed, 7 Jul 2021 09:48:20 GMT, Jeanette Winzenburg wrote: >> The issue is about memory leaks and side-effects (like NPEs) when switching >> skins. >> >> Details (copied from issue for convenience): >> >> memory leak in TextInputControlBehavior: >> - listener accidentally added twice

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin [v2]

2021-07-08 Thread Ambarish Rapte
On Wed, 7 Jul 2021 10:02:52 GMT, Jeanette Winzenburg wrote: >> modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/behavior/BehaviorCleanupTest.java >> line 404: >> >>> 402: } >>> 403: if (!root.getChildren().contains(control)) { >>> 404:

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin [v2]

2021-07-07 Thread Jeanette Winzenburg
On Tue, 6 Jul 2021 20:00:16 GMT, Ambarish Rapte wrote: >> Jeanette Winzenburg has updated the pull request incrementally with one >> additional commit since the last revision: >> >> addressed review issues > >

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin [v2]

2021-07-07 Thread Jeanette Winzenburg
On Tue, 6 Jul 2021 20:21:09 GMT, Ambarish Rapte wrote: >> I'm also interested in the opinion from others. I think we are a bit more >> safer with weak listener and there are used often as well. >> But as you correctly mentioned a lot of times (still) a listener is created >> inline. But I

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin [v2]

2021-07-07 Thread Jeanette Winzenburg
> The issue is about memory leaks and side-effects (like NPEs) when switching > skins. > > Details (copied from issue for convenience): > > memory leak in TextInputControlBehavior: > - listener accidentally added twice (removed once) > - keyPad mappings not properly installed/disposed > >

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin

2021-07-06 Thread Ambarish Rapte
On Thu, 17 Jun 2021 12:41:56 GMT, Jeanette Winzenburg wrote: > The issue is about memory leaks and side-effects (like NPEs) when switching > skins. > > Details (copied from issue for convenience): > > memory leak in TextInputControlBehavior: > - listener accidentally added twice (removed

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin

2021-07-06 Thread Ambarish Rapte
On Fri, 2 Jul 2021 22:58:02 GMT, Marius Hanl wrote: >> Okay, went through listener registrations in all behaviors - and they are >> indeed inconsistent: >> >> - some listen to control properties like focused (f.i. Button, Combo): >> adding strong, often inline listeners >> - some listen to

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin

2021-07-02 Thread Marius Hanl
On Tue, 29 Jun 2021 12:36:50 GMT, Jeanette Winzenburg wrote: >> we are a bit inconsistent in wrapping (or not) listeners into their weak >> counterparts - behaviors tend to not :) That's okay - and less crowded by >> additional code - as long as they are removed properly, IMO. But just >>

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin

2021-06-29 Thread Jeanette Winzenburg
On Mon, 28 Jun 2021 15:42:47 GMT, Jeanette Winzenburg wrote: >> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextFieldBehavior.java >> line 75: >> >>> 73: } >>> 74: >>> 75: focusListener = (observable, oldValue, newValue) -> { >> >> Shouldn't

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin

2021-06-28 Thread Jeanette Winzenburg
On Mon, 28 Jun 2021 13:18:38 GMT, Marius Hanl wrote: >> The issue is about memory leaks and side-effects (like NPEs) when switching >> skins. >> >> Details (copied from issue for convenience): >> >> memory leak in TextInputControlBehavior: >> - listener accidentally added twice (removed

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin

2021-06-28 Thread Jeanette Winzenburg
On Mon, 28 Jun 2021 12:55:47 GMT, Marius Hanl wrote: >> The issue is about memory leaks and side-effects (like NPEs) when switching >> skins. >> >> Details (copied from issue for convenience): >> >> memory leak in TextInputControlBehavior: >> - listener accidentally added twice (removed

Re: RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin

2021-06-28 Thread Marius Hanl
On Thu, 17 Jun 2021 12:41:56 GMT, Jeanette Winzenburg wrote: > The issue is about memory leaks and side-effects (like NPEs) when switching > skins. > > Details (copied from issue for convenience): > > memory leak in TextInputControlBehavior: > - listener accidentally added twice (removed

RFR: 8240506: TextFieldSkin/Behavior: misbehavior on switching skin

2021-06-17 Thread Jeanette Winzenburg
The issue is about memory leaks and side-effects (like NPEs) when switching skins. Details (copied from issue for convenience): memory leak in TextInputControlBehavior: - listener accidentally added twice (removed once) - keyPad mappings not properly installed/disposed memory leak