Re: [Integrated] RFR: 8242507: Add support for Visual Studio 2019

2020-05-05 Thread Kevin Rushforth
On Wed, 22 Apr 2020 18:46:29 GMT, Kevin Rushforth wrote: > This PR adds support for Microsoft Visual Studio 2019 (VS 2019), but does not > change the default compiler, which > remains at VS 2017 15.9.16. Changing the build compiler to VS 2019 will be > proposed in a future PR using a new bug

Re: [Rev 02] RFR: 8244297: memory leak test utility

2020-05-05 Thread Florian Kirmaier
On Tue, 5 May 2020 10:57:23 GMT, Florian Kirmaier wrote: >> I would suggest making everything configurable in the original library which >> will be suggested here to be changed. >> I will now add the possibility to make the creation of the heapDump >> configurable, and also the createGarbage

Re: [Rev 02] RFR: 8244297: memory leak test utility

2020-05-05 Thread Florian Kirmaier
> It's based on the discussion of my previous PR: > https://github.com/openjdk/jfx/pull/71 > > I Added test utility class copied from JMemoryBuddy and used it to simplify 4 > of the existing unit tests. > > It's a direct copy of my project >

RFR: 8244421: Wrong scrollbar position on touch enabled devices

2020-05-05 Thread Jose Pereda
`VirtualFlow` makes use of `VirtualScrollBar` controls, that are laid out next to the clipped container region, by default. However, when touch is supported, these scrollBars are floating controls laid out over the container. Therefore, in this case, when the viewport dimensions are updated,

Re: [Closed] RFR: 8171898: [WebView] ScrollBarWidget thickness calculation is not correct

2020-05-05 Thread Arun Joseph
On Mon, 27 Apr 2020 08:17:50 GMT, Arun Joseph wrote: > ScrollbarThemeJava delegates scrollbar thickness calculation to > ScrollBarWidget::initializeThickness() > [ScrollBarWidget.java] method. Since "ScrollbarThemeJava::scrollbarThickness" > is not associated with any > "ScrollBarWidget"

Re: [Integrated] RFR: 8244110: NPE in MenuButtonSkinBase change listener

2020-05-05 Thread Ajit Ghaisas
On Mon, 4 May 2020 09:07:53 GMT, Ajit Ghaisas wrote: > Issue : > https://bugs.openjdk.java.net/browse/JDK-8244110 > > Root Cause : > Fix of [JDK-8175358](https://bugs.openjdk.java.net/browse/JDK-8175358) added > code to remove accelerators from a scene > in Scene property listener of

Re: [Rev 01] RFR: 8244110: NPE in MenuButtonSkinBase change listener

2020-05-05 Thread Jeanette Winzenburg
On Tue, 5 May 2020 11:07:43 GMT, Ajit Ghaisas wrote: >> Issue : >> https://bugs.openjdk.java.net/browse/JDK-8244110 >> >> Root Cause : >> Fix of [JDK-8175358](https://bugs.openjdk.java.net/browse/JDK-8175358) added >> code to remove accelerators from a scene >> in Scene property listener of

Re: [Rev 01] RFR: 8244110: NPE in MenuButtonSkinBase change listener

2020-05-05 Thread Kevin Rushforth
On Tue, 5 May 2020 13:53:02 GMT, Jeanette Winzenburg wrote: > > > not sure if it is appropriate to approve this? we talked so much about it, > that it feels like self-approval :) Please do approve it. And thank you for contributing the solution and test.  - PR:

Re: [Rev 01] RFR: 8244110: NPE in MenuButtonSkinBase change listener

2020-05-05 Thread Jeanette Winzenburg
On Tue, 5 May 2020 12:38:26 GMT, Kevin Rushforth wrote: >> Ajit Ghaisas has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Make ChangeListener Final > > Looks good. I confirm that the original system test and the new controls unit > test

Re: RFR: 8242507: Add support for Visual Studio 2019

2020-05-05 Thread Johan Vos
On Wed, 22 Apr 2020 18:46:29 GMT, Kevin Rushforth wrote: > This PR adds support for Microsoft Visual Studio 2019 (VS 2019), but does not > change the default compiler, which > remains at VS 2017 15.9.16. Changing the build compiler to VS 2019 will be > proposed in a future PR using a new bug

Re: [Rev 01] RFR: 8244110: NPE in MenuButtonSkinBase change listener

2020-05-05 Thread Kevin Rushforth
On Tue, 5 May 2020 11:07:43 GMT, Ajit Ghaisas wrote: >> Issue : >> https://bugs.openjdk.java.net/browse/JDK-8244110 >> >> Root Cause : >> Fix of [JDK-8175358](https://bugs.openjdk.java.net/browse/JDK-8175358) added >> code to remove accelerators from a scene >> in Scene property listener of

Re: [Rev 01] RFR: 8244110: NPE in MenuButtonSkinBase change listener

2020-05-05 Thread Kevin Rushforth
On Tue, 5 May 2020 10:35:12 GMT, Ajit Ghaisas wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuButtonSkinBase.java >> line 230: >> >>> 229: // Remove listeners >>> 230: >>> getSkinnable().sceneProperty().removeListener(sceneChangeListener); >>>

Re: [Rev 01] RFR: 8244297: memory leak test utility

2020-05-05 Thread Florian Kirmaier
> It's based on the discussion of my previous PR: > https://github.com/openjdk/jfx/pull/71 > > I Added test utility class copied from JMemoryBuddy and used it to simplify 4 > of the existing unit tests. > > It's a direct copy of my project >

Re: [Rev 01] RFR: 8244297: memory leak test utility

2020-05-05 Thread Florian Kirmaier
On Mon, 4 May 2020 13:47:58 GMT, Kevin Rushforth wrote: >> Florian Kirmaier has updated the pull request incrementally with one >> additional commit since the last revision: >> >> JDK-8244297 >> Updated JMemoeryBuddy >> Added Copyright header >> disabled default heapdump > >

Re: [Rev 01] RFR: 8244110: NPE in MenuButtonSkinBase change listener

2020-05-05 Thread Ajit Ghaisas
> Issue : > https://bugs.openjdk.java.net/browse/JDK-8244110 > > Root Cause : > Fix of [JDK-8175358](https://bugs.openjdk.java.net/browse/JDK-8175358) added > code to remove accelerators from a scene > in Scene property listener of MenuButtonSkinBase. That fix uses > getSkinnable() in listener.

Re: [Rev 01] RFR: 8244110: NPE in MenuButtonSkinBase change listener

2020-05-05 Thread Ajit Ghaisas
On Mon, 4 May 2020 15:46:57 GMT, Kevin Rushforth wrote: >> Ajit Ghaisas has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Make ChangeListener Final > >

Re: RFR: 8244297: memory leak test utility

2020-05-05 Thread Florian Kirmaier
On Tue, 5 May 2020 10:38:56 GMT, Florian Kirmaier wrote: >> I'll put this on my queue to review and test it. I left some high-level >> comments below. > > I would suggest making everything configurable in the original library which > will be suggested here to be changed. > I will now add the

RFR: 8242001: ChoiceBox: must update value on setting SelectionModel, part2

2020-05-05 Thread Jeanette Winzenburg
The issue is that ChoiceBox didn't sync its value to the replaced SelectionModel's selectedItem if the item is null. Fixed by removing the constraint from selectionModel property. Added tests that failed before and passed after the fix. Note that I also changed a test method in ChoiceBoxTest

Re: RFR: 8244297: memory leak test utility

2020-05-05 Thread Florian Kirmaier
On Mon, 4 May 2020 14:24:48 GMT, Kevin Rushforth wrote: >> It's based on the discussion of my previous PR: >> https://github.com/openjdk/jfx/pull/71 >> >> I Added test utility class copied from JMemoryBuddy and used it to simplify >> 4 of the existing unit tests. >> >> It's a direct copy of

Re: [Integrated] RFR: 8241999: ChoiceBox: incorrect toggle selected for uncontained

2020-05-05 Thread Jeanette Winzenburg
On Tue, 28 Apr 2020 15:36:53 GMT, Jeanette Winzenburg wrote: > The issue is that the toggles is not reliably unselected if an uncontained > value is set. > > The root is ChoiceBoxSelectionModel which doesn't update the index on > selecting an uncontained item, in particular it > fails to