Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v5]

2022-12-06 Thread Karthik P K
On Tue, 6 Dec 2022 23:21:55 GMT, Kevin Rushforth wrote: >> Karthik P K has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Address review comment. Add comments for changes > >

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v5]

2022-12-06 Thread Ajit Ghaisas
On Tue, 6 Dec 2022 23:46:47 GMT, Andy Goryachev wrote: >> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR >> - fixing memory leaks in SpinnerSkin by properly removing all listeners and >> event filters added in the constructor, as well as any child Nodes. >> >>

Integrated: 8296854: NULL check of CTFontCopyAvailableTables return value is required

2022-12-06 Thread Phil Race
On Mon, 5 Dec 2022 21:18:38 GMT, Phil Race wrote: > Guard against de-referencing null, although it is currently theoretical as > far as I can see (more info in the bug eval) This pull request has now been integrated. Changeset: f96b3504 Author:Phil Race URL:

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v5]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 23:39:51 GMT, Andy Goryachev wrote: >> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR >> - fixing memory leaks in SpinnerSkin by properly removing all listeners and >> event filters added in the constructor, as well as any child Nodes. >> >>

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v5]

2022-12-06 Thread Andy Goryachev
> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR - > fixing memory leaks in SpinnerSkin by properly removing all listeners and > event filters added in the constructor, as well as any child Nodes. > > NOTE: the fix requires both ListenerHelper >

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v4]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 17:18:50 GMT, Andy Goryachev wrote: >> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR >> - fixing memory leaks in SpinnerSkin by properly removing all listeners and >> event filters added in the constructor, as well as any child Nodes. >> >>

Re: RFR: 8296854: NULL check of CTFontCopyAvailableTables return value is required

2022-12-06 Thread Kevin Rushforth
On Mon, 5 Dec 2022 21:18:38 GMT, Phil Race wrote: > Guard against de-referencing null, although it is currently theoretical as > far as I can see (more info in the bug eval) Marked as reviewed by kcr (Lead). - PR: https://git.openjdk.org/jfx/pull/968

Re: RFR: 8296621: Stage steals focus on scene change [v6]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 17:38:45 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v5]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 11:25:44 GMT, Karthik P K wrote: >> Cause: When slider is dragged for first time after tooltip appears, >> setOnMousePressed event was not invoked, hence dragStart was null in the >> subsequently invoked event handler (setOnMouseDragged). >> >> Fix: Initialized dragStart in

Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v7]

2022-12-06 Thread Kevin Rushforth
On Sat, 7 May 2022 04:17:13 GMT, John Neffenger wrote: >> This pull request allows for reproducible builds of JavaFX on Linux, macOS, >> and Windows by defining the `SOURCE_DATE_EPOCH` environment variable. For >> example, the following commands create a reproducible build: >> >> >> $ export

Re: RFR: 8289357: (Tree)TableView is null in (Tree)TableRowSkin during autosize [v6]

2022-12-06 Thread Kevin Rushforth
On Wed, 10 Aug 2022 08:14:13 GMT, Marius Hanl wrote: >> Initialize the `(Tree)TableView` when creating the measure row. >> This will guarantee, that we can access the `(Tree)TableView` in the >> `(Tree)TableRowSkin`, which is currently only null during the autosizing (It >> is always set

Re: RFR: 8289357: (Tree)TableView is null in (Tree)TableRowSkin during autosize [v6]

2022-12-06 Thread Andy Goryachev
On Wed, 10 Aug 2022 08:14:13 GMT, Marius Hanl wrote: >> Initialize the `(Tree)TableView` when creating the measure row. >> This will guarantee, that we can access the `(Tree)TableView` in the >> `(Tree)TableRowSkin`, which is currently only null during the autosizing (It >> is always set

Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v7]

2022-12-06 Thread John Neffenger
On Tue, 6 Dec 2022 18:32:19 GMT, Johan Vos wrote: > Speaking of which, that is my main worry about this PR: we are increasing our > dependency on Gradle by adding logic inside the build.gradle file. You mean the 17-line `setFileTimestamps` method? I see that as the main issue, as all the

RFR: JDK-8298200 Clean up raw type warnings in javafx.beans.property.* and com.sun.javafx.property.*

2022-12-06 Thread John Hendrikx
- Added generics (to package private or internal classes only) - Minor clean-ups of code I touched (naming) - Fixed incorrect use of generics - Fixed raw type warnings Note: some raw types have leaked into public API. These could be fixed without incompatibilities. For specifics see

Re: RFR: JDK-8298200 Clean up raw type warnings in javafx.beans.property.* and com.sun.javafx.property.*

2022-12-06 Thread John Hendrikx
On Tue, 6 Dec 2022 18:12:39 GMT, John Hendrikx wrote: > - Added generics (to package private or internal classes only) > - Minor clean-ups of code I touched (naming) > - Fixed incorrect use of generics > - Fixed raw type warnings > > Note: some raw types have leaked into public API. These

Re: RFR: 8289357: (Tree)TableView is null in (Tree)TableRowSkin during autosize [v6]

2022-12-06 Thread Andy Goryachev
On Wed, 10 Aug 2022 08:14:13 GMT, Marius Hanl wrote: >> Initialize the `(Tree)TableView` when creating the measure row. >> This will guarantee, that we can access the `(Tree)TableView` in the >> `(Tree)TableRowSkin`, which is currently only null during the autosizing (It >> is always set

Re: RFR: 8289357: (Tree)TableView is null in (Tree)TableRowSkin during autosize [v6]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 18:38:17 GMT, Marius Hanl wrote: > Can anyone else check this out soon? :) Sorry about the delay. I do want to see this go in soon, so yes, I can take a look. Perhaps @andy-goryachev-oracle can too? - PR: https://git.openjdk.org/jfx/pull/805

Re: RFR: 8289357: (Tree)TableView is null in (Tree)TableRowSkin during autosize [v6]

2022-12-06 Thread Marius Hanl
On Wed, 10 Aug 2022 08:14:13 GMT, Marius Hanl wrote: >> Initialize the `(Tree)TableView` when creating the measure row. >> This will guarantee, that we can access the `(Tree)TableView` in the >> `(Tree)TableRowSkin`, which is currently only null during the autosizing (It >> is always set

Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v7]

2022-12-06 Thread Johan Vos
On Sat, 7 May 2022 04:17:13 GMT, John Neffenger wrote: >> This pull request allows for reproducible builds of JavaFX on Linux, macOS, >> and Windows by defining the `SOURCE_DATE_EPOCH` environment variable. For >> example, the following commands create a reproducible build: >> >> >> $ export

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v4]

2022-12-06 Thread Andy Goryachev
On Tue, 6 Dec 2022 17:18:50 GMT, Andy Goryachev wrote: >> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR >> - fixing memory leaks in SpinnerSkin by properly removing all listeners and >> event filters added in the constructor, as well as any child Nodes. >> >>

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v4]

2022-12-06 Thread Ajit Ghaisas
On Tue, 6 Dec 2022 17:18:50 GMT, Andy Goryachev wrote: >> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR >> - fixing memory leaks in SpinnerSkin by properly removing all listeners and >> event filters added in the constructor, as well as any child Nodes. >> >>

Re: RFR: 8264449: Enable reproducible builds with SOURCE_DATE_EPOCH [v7]

2022-12-06 Thread John Neffenger
On Sat, 7 May 2022 04:17:13 GMT, John Neffenger wrote: >> This pull request allows for reproducible builds of JavaFX on Linux, macOS, >> and Windows by defining the `SOURCE_DATE_EPOCH` environment variable. For >> example, the following commands create a reproducible build: >> >> >> $ export

Re: RFR: 8266396: Save VSCMD_DEBUG output in Windows build [v3]

2022-12-06 Thread John Neffenger
On Tue, 11 May 2021 01:17:32 GMT, John Neffenger wrote: >> The Windows build calls a series of batch files to get the Visual Studio >> paths and environment variables. The batch files are a black box: any >> messages they print are discarded. If anything goes wrong, the only signs >> are a

Withdrawn: 8266396: Save VSCMD_DEBUG output in Windows build

2022-12-06 Thread John Neffenger
On Thu, 6 May 2021 20:39:11 GMT, John Neffenger wrote: > The Windows build calls a series of batch files to get the Visual Studio > paths and environment variables. The batch files are a black box: any > messages they print are discarded. If anything goes wrong, the only signs are > a vague

Re: RFR: 8296621: Stage steals focus on scene change [v4]

2022-12-06 Thread Thiago Milczarek Sayao
On Tue, 6 Dec 2022 13:09:25 GMT, Kevin Rushforth wrote: >> I don't mind changing it, but I think those durations are relative, so >> `Duration.millis(0)` means "don't wait". I'm not sure I follow the >> recommendation. > > To expand on what Ambarish said, if you have a repeating timeline, any

Re: RFR: 8296621: Stage steals focus on scene change [v5]

2022-12-06 Thread Thiago Milczarek Sayao
On Tue, 6 Dec 2022 14:27:55 GMT, Kevin Rushforth wrote: > I still need to do a little more testing, but so far, the fix looks good to > me. > > As for the test, I left an inline comment answering your question. More > importantly, though, the test doesn't catch the bug. It passes both with

Re: RFR: 8296621: Stage steals focus on scene change [v6]

2022-12-06 Thread Thiago Milczarek Sayao
> Simple fix to not requestFocus() on scene change. > > The attached bug sample shows that the TextField focused on the scene remains > focused when the scene comes back. Thiago Milczarek Sayao has updated the pull request incrementally with one additional commit since the last revision:

Discussion: Reproducible builds

2022-12-06 Thread John Neffenger
This message continues the conversation I started with Johan in a random GitHub issue. :-) I'm posting it to the mailing list so that anyone can join the discussion with other questions or comments. We're now using the github issue tracker to discuss reproducible builds, which is probably

Re: RFR: 8295809: TreeTableViewSkin: memory leak when changing skin [v2]

2022-12-06 Thread Andy Goryachev
> as determined by SkinMemoryLeakTest (remove line 180) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > caused by: > - adding and not removing listeners > - adding and not removing event handlers/filters > - adding and not removing

Re: RFR: 8295809: TreeTableViewSkin: memory leak when changing skin

2022-12-06 Thread Andy Goryachev
On Mon, 24 Oct 2022 19:06:26 GMT, Andy Goryachev wrote: > as determined by SkinMemoryLeakTest (remove line 180) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > caused by: > - adding and not removing listeners > - adding and not

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v3]

2022-12-06 Thread Andy Goryachev
On Tue, 6 Dec 2022 11:20:51 GMT, Ajit Ghaisas wrote: >> Andy Goryachev has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 51 commits: >> >> - Merge remote-tracking branch 'origin/master' into 8245145.spinner.skin >> - Merge

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v4]

2022-12-06 Thread Andy Goryachev
> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR - > fixing memory leaks in SpinnerSkin by properly removing all listeners and > event filters added in the constructor, as well as any child Nodes. > > NOTE: the fix requires both ListenerHelper >

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v18]

2022-12-06 Thread Andy Goryachev
On Tue, 6 Dec 2022 16:41:05 GMT, Andy Goryachev wrote: >> The current CONSTRAINED_RESIZE_POLICY has a number of issues as explained in >> [JDK-8292810](https://bugs.openjdk.org/browse/JDK-8292810). >> >> We propose to address all these issues by replacing the old column resize >> algorithm

Re: RFR: 8297554: Remove Scene.KeyHandler [v3]

2022-12-06 Thread Michael Strauß
> The `Scene.KeyHandler` class doesn't seem to have a clear purpose, mixing > focus handling with event propagation. Since #852, > `KeyHandler.setFocusVisible` is also called from mouse and touch event > handlers, which makes the purpose of the class even less pronounced. > > Moving the

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v18]

2022-12-06 Thread Andy Goryachev
> The current CONSTRAINED_RESIZE_POLICY has a number of issues as explained in > [JDK-8292810](https://bugs.openjdk.org/browse/JDK-8292810). > > We propose to address all these issues by replacing the old column resize > algorithm with a different one, which not only honors all the constraints

Integrated: 8295175: SplitPaneSkin: memory leak when changing skin

2022-12-06 Thread Andy Goryachev
On Tue, 11 Oct 2022 22:44:10 GMT, Andy Goryachev wrote: > Fixed memory leak by removing all the listeners in dispose(); > > This PR depends on a new internal class ListenerHelper, a replacement for > LambdaMultiplePropertyChangeListenerHandler. > See https://github.com/openjdk/jfx/pull/908

Re: RFR: 8297554: Remove Scene.KeyHandler [v2]

2022-12-06 Thread Michael Strauß
> The `Scene.KeyHandler` class doesn't seem to have a clear purpose, mixing > focus handling with event propagation. Since #852, > `KeyHandler.setFocusVisible` is also called from mouse and touch event > handlers, which makes the purpose of the class even less pronounced. > > Moving the

Integrated: 8295531: ComboBoxBaseSkin: memory leak when changing skin

2022-12-06 Thread Andy Goryachev
On Tue, 18 Oct 2022 23:41:05 GMT, Andy Goryachev wrote: > as determined by SkinMemoryLeakTest (remove line 166) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > Affected skins: > - ColorPicker > - DatePicker > - ComboBox > >

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v5]

2022-12-06 Thread Andy Goryachev
On Tue, 6 Dec 2022 11:25:44 GMT, Karthik P K wrote: >> Cause: When slider is dragged for first time after tooltip appears, >> setOnMousePressed event was not invoked, hence dragStart was null in the >> subsequently invoked event handler (setOnMouseDragged). >> >> Fix: Initialized dragStart in

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-06 Thread Andy Goryachev
On Tue, 6 Dec 2022 09:00:02 GMT, John Hendrikx wrote: >>> This is one giant pull request, somewhat difficult on the reviewers, >>> especially since we have to go through it several times. >>> >>> It would be _much_ easier for the reviewers to deal with one fix per PR, >>> especially in the

Re: RFR: 8296621: Stage steals focus on scene change [v5]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 11:35:35 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request

Re: RFR: 8296621: Stage steals focus on scene change [v4]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 11:01:08 GMT, Thiago Milczarek Sayao wrote: >> tests/system/src/test/java/test/robot/javafx/scene/SceneChangeShouldNotFocusStageTest.java >> line 80: >> >>> 78: tl.setCycleCount(Animation.INDEFINITE); >>> 79: tl.getKeyFrames().addAll(new

Re: RFR: 8296621: Stage steals focus on scene change [v5]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 11:35:35 GMT, Thiago Milczarek Sayao wrote: >> Simple fix to not requestFocus() on scene change. >> >> The attached bug sample shows that the TextField focused on the scene >> remains focused when the scene comes back. > > Thiago Milczarek Sayao has updated the pull request

Integrated: JDK-8297413: Remove easy warnings in javafx.graphics

2022-12-06 Thread John Hendrikx
On Tue, 22 Nov 2022 18:39:43 GMT, John Hendrikx wrote: > - Remove unsupported/unnecessary SuppressWarning annotations > - Remove reduntant type specifications (use diamond operator) > - Remove unused or duplicate imports > - Remove unnecessary casts (type is already correct type or can be

Integrated: 8295324: JavaFX: Blank pages when printing

2022-12-06 Thread eduardsdv
On Fri, 14 Oct 2022 15:45:10 GMT, eduardsdv wrote: > This fixes a race condition between application and 'Print Job Thread' > threads when printing. > > The race condition occurs when application thread calls `endJob()`, which in > effect sets the `jobDone` flag to true, > and when the 'Print

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 08:54:26 GMT, John Hendrikx wrote: >> I don't see it as a useful warning. The code isn't necessarily better >> without, and can be less clear. I think this might argue for asking Eclipse >> users to disable this warning in the IDE. > > @kevinrushforth Is this okay as it is?

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 06:39:32 GMT, Nir Lisker wrote: >> Since this is an internal class (not public API), I won't object to this >> change. I do think it's a rather pointless change, but I'll leave it up to >> you. > > Personally, I find that re-declaring type hierarchy is clutter. I can see why

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-06 Thread Kevin Rushforth
On Tue, 6 Dec 2022 06:33:20 GMT, Nir Lisker wrote: >> I agree with you - we shouldn't. > > This is an example of what I was talking about in [this > comment](https://github.com/openjdk/jfx/pull/958#pullrequestreview-1198517697). I agree that no follow-on bug is needed. - PR:

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-06 Thread Kevin Rushforth
On Sat, 3 Dec 2022 20:54:04 GMT, John Hendrikx wrote: >> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-06 Thread Kevin Rushforth
On Sat, 3 Dec 2022 20:46:22 GMT, John Hendrikx wrote: >> modules/javafx.graphics/src/main/java/com/sun/glass/ui/monocle/LinuxStatefulMultiTouchProcessor.java >> line 42: >> >>> 40: >>> 41: private final Map slotToIDMap = >>> 42: new HashMap<>(); >> >> Same line. There are

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v4]

2022-12-06 Thread Ajit Ghaisas
On Tue, 6 Dec 2022 11:21:08 GMT, Karthik P K wrote: > @aghaisas please check if bug given above is fixed. I tested the program provided in [JDK-8190411](https://bugs.openjdk.org/browse/JDK-8190411) with touch based Windows system. The proposed fix fixes the issue. For

Re: RFR: 8260528: Clean glass-gtk sizing and positioning code [v30]

2022-12-06 Thread Thiago Milczarek Sayao
On Tue, 22 Nov 2022 01:40:02 GMT, Thiago Milczarek Sayao wrote: >> This cleans size and positioning code, reducing special cases, code >> complexity and size. >> >> Changes: >> >> - cached extents: 28, 1, 1, 1 are old defaults - modern gnome uses different >> sizes. It does not assume any

Re: RFR: 8295531: ComboBoxBaseSkin: memory leak when changing skin [v5]

2022-12-06 Thread Ajit Ghaisas
On Fri, 2 Dec 2022 17:35:05 GMT, Andy Goryachev wrote: >> as determined by SkinMemoryLeakTest (remove line 166) and a leak tester >> https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java >> >> Affected skins: >> - ColorPicker >> - DatePicker >> - ComboBox >>

Re: RFR: 8296621: Stage steals focus on scene change [v4]

2022-12-06 Thread Thiago Milczarek Sayao
On Tue, 6 Dec 2022 05:09:40 GMT, Ambarish Rapte wrote: >> Thiago Milczarek Sayao has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add new line > > tests/system/src/test/java/test/robot/javafx/scene/SceneChangeShouldNotFocusStageTest.java

Re: RFR: 8296621: Stage steals focus on scene change [v5]

2022-12-06 Thread Thiago Milczarek Sayao
> Simple fix to not requestFocus() on scene change. > > The attached bug sample shows that the TextField focused on the scene remains > focused when the scene comes back. Thiago Milczarek Sayao has updated the pull request with a new target base due to a merge or a rebase. The incremental

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v4]

2022-12-06 Thread Karthik P K
On Mon, 5 Dec 2022 12:48:14 GMT, Karthik P K wrote: >> Cause: When slider is dragged for first time after tooltip appears, >> setOnMousePressed event was not invoked, hence dragStart was null in the >> subsequently invoked event handler (setOnMouseDragged). >> >> Fix: Initialized dragStart in

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v4]

2022-12-06 Thread Karthik P K
On Mon, 5 Dec 2022 17:16:01 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/SliderSkin.java >> line 395: >> >>> 393: >>> 394: thumb.setOnMouseEntered(me -> { >>> 395: if (getSkinnable().getTooltip() != null && >>>

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v4]

2022-12-06 Thread Karthik P K
On Mon, 5 Dec 2022 17:18:12 GMT, Andy Goryachev wrote: >> Karthik P K has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update in mouse enter and exit events. Update in unit test > >

Re: RFR: 8245145: Spinner: throws IllegalArgumentException when replacing skin [v3]

2022-12-06 Thread Ajit Ghaisas
On Fri, 2 Dec 2022 17:42:00 GMT, Andy Goryachev wrote: >> Fixed SpinnerSkin initialization using new Skin.install(). Also in this PR >> - fixing memory leaks in SpinnerSkin by properly removing all listeners and >> event filters added in the constructor, as well as any child Nodes. >> >>

Re: RFR: 8190411: NPE in SliderSkin:140 if Slider.Tooltip.autohide is true [v5]

2022-12-06 Thread Karthik P K
> Cause: When slider is dragged for first time after tooltip appears, > setOnMousePressed event was not invoked, hence dragStart was null in the > subsequently invoked event handler (setOnMouseDragged). > > Fix: Initialized dragStart in initialize method. > > Test: Added system test to

Re: RFR: 8295175: SplitPaneSkin: memory leak when changing skin [v5]

2022-12-06 Thread Ajit Ghaisas
On Mon, 5 Dec 2022 16:49:43 GMT, Andy Goryachev wrote: >> Fixed memory leak by removing all the listeners in dispose(); >> >> This PR depends on a new internal class ListenerHelper, a replacement for >> LambdaMultiplePropertyChangeListenerHandler. >> See https://github.com/openjdk/jfx/pull/908

Re: RFR: 8296621: Stage steals focus on scene change [v4]

2022-12-06 Thread Thiago Milczarek Sayao
On Tue, 6 Dec 2022 05:35:07 GMT, Ambarish Rapte wrote: >> Thiago Milczarek Sayao has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Add new line > > tests/system/src/test/java/test/robot/javafx/scene/SceneChangeShouldNotFocusStageTest.java

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-12-06 Thread John Hendrikx
On Wed, 30 Nov 2022 12:48:10 GMT, Kevin Rushforth wrote: >> If the casts in the numerator actually matter, then the cast in the >> denominator can be removed. The latter are the ones that Eclipse flags for >> me as unnecessary. > > I still think that any change here would be a very low value

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-06 Thread John Hendrikx
On Mon, 5 Dec 2022 22:20:59 GMT, John Hendrikx wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix indentations and merge short lines > >> This is one giant pull request, somewhat difficult on the reviewers, >>

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v4]

2022-12-06 Thread John Hendrikx
On Tue, 22 Nov 2022 22:30:16 GMT, Kevin Rushforth wrote: >> I agree. The problem is that we will not be able to enable the warning in >> IDE, or it has to be @suppressed. >> >> So the choice is either fix the code and enable warning, or keep the code as >> is and disable the warning. > > I

Re: RFR: 8297554: Remove Scene.KeyHandler

2022-12-06 Thread Ambarish Rapte
On Thu, 24 Nov 2022 06:49:02 GMT, Michael Strauß wrote: > The `Scene.KeyHandler` class doesn't seem to have a clear purpose, mixing > focus handling with event propagation. Since #852, > `KeyHandler.setFocusVisible` is also called from mouse and touch event > handlers, which makes the purpose