Re: RFR: 8268225: Support :focus-visible and :focus-within CSS pseudoclasses

2021-06-07 Thread Marius Hanl
On Wed, 21 Apr 2021 18:17:27 GMT, Michael Strauß wrote: > This PR adds the `Node.focusVisible` and `Node.focusWithin` properties, as > well as the corresponding `:focus-visible` and `:focus-within` CSS > pseudo-classes. Do you have a example application or something, where the possible use

Re: RFR: 8267418: IntelliJ build and test of JavaFX does not work [v4]

2021-06-10 Thread Marius Hanl
On Mon, 7 Jun 2021 04:17:06 GMT, Ambarish Rapte wrote: > I tested with IntelliJ IDEA 2021.1.2 (Community Edition) Build > #IC-211.7442.40, built on June 1, 2021. > > I see no side effects of this change. However, I also don't see what you see > too. > I tried it with a fresh clone of

Integrated: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability

2021-06-23 Thread Marius Hanl
On Sun, 6 Jun 2021 12:44:00 GMT, Marius Hanl wrote: > This PR enables Tree- and TableCells to also check the row editability when > an edit should happen. With this a Tree- or TableCell is not editable, when > the row where the cell is in is not. > > While this PR fixes the pr

Re: RFR: 8264137: Suppress deprecation and removal warnings of internal methods [v2]

2021-06-23 Thread Marius Hanl
On Wed, 23 Jun 2021 11:16:09 GMT, Ajit Ghaisas wrote: >> This PR suppresses -PLINT="removal" warnings and fixes -PLINT="deprecation" >> warnings. I have created separate commits for each type of warnings for the >> ease of review. >> >> `gradle --info -PLINT="deprecation,removal"` -- results

Re: RFR: 8267418: IntelliJ build and test of JavaFX does not work [v4]

2021-06-22 Thread Marius Hanl
On Tue, 15 Jun 2021 17:44:43 GMT, Ambarish Rapte wrote: > ``` > diff --git a/gradle/verification-metadata.xml > b/gradle/verification-metadata.xml > index abacd0b05b..0a3d33726d 100644 > --- a/gradle/verification-metadata.xml > +++ b/gradle/verification-metadata.xml > @@ -3,6 +3,9 @@ > >

Re: RFR: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability [v2]

2021-06-22 Thread Marius Hanl
On Fri, 18 Jun 2021 10:14:40 GMT, Jeanette Winzenburg wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/TableCell.java >> line 310: >> >>> 308: (table != null && !table.isEditable()) || >>> 309: (column != null && !column.isEditable()) || >>>

Re: RFR: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability [v2]

2021-06-22 Thread Marius Hanl
On Fri, 18 Jun 2021 10:01:42 GMT, Ajit Ghaisas wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Addressed review comments > > modules/javafx.controls/src/test/java/test/javafx/sc

Re: RFR: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability [v2]

2021-06-22 Thread Marius Hanl
On Fri, 18 Jun 2021 10:10:43 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Addressed review comments > > modules/javafx.controls/src/test/

Re: RFR: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability [v2]

2021-06-22 Thread Marius Hanl
count the row editability in: > [JDK-8268295](https://bugs.openjdk.java.net/browse/JDK-8268295) Marius Hanl has updated the pull request incrementally with one additional commit since the last revision: Addressed review comments - Changes: - all: https://git.openjdk.java.net/jf

Re: RFR: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability

2021-06-22 Thread Marius Hanl
On Fri, 18 Jun 2021 10:23:33 GMT, Jeanette Winzenburg wrote: > > I created a follow-up issues for fixing all the sub Tree- and TableCell > > implementation which do not count the row editability in: > > [JDK-8268295](https://bugs.openjdk.java.net/browse/JDK-8268295) > > as already commented

Re: RFR: 8264137: Suppress deprecation and removal warnings of internal methods

2021-06-22 Thread Marius Hanl
On Mon, 21 Jun 2021 11:24:39 GMT, Ajit Ghaisas wrote: > This PR suppresses -PLINT="removal" warnings and fixes -PLINT="deprecation" > warnings. I have created separate commits for each type of warnings for the > ease of review. > > `gradle --info -PLINT="deprecation,removal"` -- results in

Integrated: 8186904: TableColumnHeader: resize cursor lost on right click

2021-05-17 Thread Marius Hanl
On Wed, 28 Apr 2021 20:26:35 GMT, Marius Hanl wrote: > This PR is fixing a bug, when a right click on a table column is performed. > By doing that, the table columns will lose the resize cursor thus they can > not be resized anymore. > > The reason for that is that the *

Integrated: 8267392: ENTER key press on editable TableView throws NPE

2021-05-21 Thread Marius Hanl
On Wed, 19 May 2021 00:48:18 GMT, Marius Hanl wrote: > ~~Note: I reported the bug already, waiting for approval. Internal tracking > id: 9070318. I will update the title as soon as the ticket is created.~~ > EDIT: Changed the title. :) > > This PR is fixing a NP, which is

Re: RFR: 8267505: {List, Set, Map}PropertyBase::bind should check against identity

2021-05-28 Thread Marius Hanl
On Mon, 24 May 2021 11:56:35 GMT, Jose Pereda wrote: > ListPropertyBase::bind, SetPropertyBase::bind, MapPropertyBase::bind have a > check on whether a different instance of the observable is the same, and this > PR changes that to check against identity. > > Tests are included. Looks good.

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

2021-05-28 Thread Marius Hanl
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

Re: RFR: 8267418: IntelliJ build and test of JavaFX does not work [v3]

2021-06-02 Thread Marius Hanl
On Tue, 1 Jun 2021 11:08:26 GMT, Ambarish Rapte wrote: > Is a specific version of IntelliJ required to test this fix, I use 2019.2 > version. I always build using terminal, on my IntelliJ the build fails both > before and after. ### I'm using the newest version (Community edition, 2021.1.2).

Re: RFR: 8267418: IntelliJ build and test of JavaFX does not work [v4]

2021-06-02 Thread Marius Hanl
normal tests with IntelliJ** > > ### What I couldn't fix yet (When I tried, it looked like IntelliJ is > overriding the settings on next gradle reload): > - IntelliJ is not detecting javafx.graphic inside the shims > - All javafx.* dependencies are not found for the system tes

Re: RFR: 8267418: IntelliJ build and test of JavaFX does not work [v3]

2021-05-29 Thread Marius Hanl
l** are modified to include/recognize JUnit 4. Also the > shims are included/recognized (as test resource, this is very similar to the > configuration inside the .classpath file from Eclipse) Marius Hanl has updated the pull request incrementally with one additional commit since the last revision:

Re: RFR: 8267418: IntelliJ build and test of JavaFX does not work [v2]

2021-05-29 Thread Marius Hanl
l** are modified to include/recognize JUnit 4. Also the > shims are included/recognized (as test resource, this is very similar to the > configuration inside the .classpath file from Eclipse) Marius Hanl has updated the pull request with a new target base due to a merge or a rebase. The

RFR: 8252238: TableView: Editable (pseudo-editable) cells should respect the row editability

2021-06-06 Thread Marius Hanl
This PR enables Tree- and TableCells to also check the row editability when an edit should happen. With this a Tree- or TableCell is not editable, when the row where the cell is in is not. While this PR fixes the problem described in the ticket, it does not fix the example. This is due the

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v4]

2021-07-07 Thread Marius Hanl
The tests checks, that no NPE is printed to the console. They also checks, > that the set value is still displayed (either in the ComboBox button cell or > the ChoiceBox display label) Marius Hanl has updated the pull request incrementally with two additional commits since the last rev

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v3]

2021-07-07 Thread Marius Hanl
On Wed, 7 Jul 2021 11:18:41 GMT, Ajit Ghaisas wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Added another cbx test + cleaned up exception handler + let the exception >> bubble up in

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v3]

2021-07-07 Thread Marius Hanl
On Wed, 7 Jul 2021 10:24:59 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Added another cbx test + cleaned up exception handler + let the exception >>

RFR: 8188026: TextFieldXXCell: NPE on calling startEdit

2021-07-07 Thread Marius Hanl
This PR sets an unified logic to every **startEdit()** method of all Cell implementations. So startEdit() is always doing the same now: `super.startEdit();` `if (!isEditing()) { return; }` This will prevent a NPE while also being cleaner (no more double checks) The commits are splitted into 4

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v3]

2021-07-07 Thread Marius Hanl
On Wed, 7 Jul 2021 11:04:27 GMT, Ajit Ghaisas wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Added another cbx test + cleaned up exception handler + let the exception >> bubble up in

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: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v2]

2021-07-03 Thread Marius Hanl
On Thu, 1 Jul 2021 15:09:43 GMT, Marius Hanl wrote: >> modules/javafx.controls/src/test/java/test/javafx/scene/control/ChoiceBoxTest.java >> line 162: >> >>> 160: ByteArrayOutputStream out = new ByteArrayOutputStream(); >>> 161: Sys

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v2]

2021-07-03 Thread Marius Hanl
The tests checks, that no NPE is printed to the console. They also checks, > that the set value is still displayed (either in the ComboBox button cell or > the ChoiceBox display label) Marius Hanl has updated the pull request incrementally with two additional commits since the last revi

Re: RFR: 8187229: Tree/TableCell: cancel event must return correct editing location

2021-07-03 Thread Marius Hanl
On Thu, 1 Jul 2021 12:38:06 GMT, Jeanette Winzenburg wrote: > the bug is an incorrect edit location (for Tree/Table: Tree/TablePosition) in > edit cancel events - expected is the location at the time the cell edit was > started, actual was the location of at the time the edit was cancelled.

Integrated: 8267418: IntelliJ build and test of JavaFX does not work

2021-06-29 Thread Marius Hanl
On Wed, 19 May 2021 16:12:45 GMT, Marius Hanl wrote: > ~~Question: I was wondering, should I create a ticket for this as well? Given > the fact that I don't have an https://bugs.openjdk.java.net account, I need > to use the official bug reporting tool, which looked a bit overk

Re: RFR: 8258499: JavaFX: Move src.zip out of the lib directory

2021-06-30 Thread Marius Hanl
On Wed, 30 Jun 2021 15:06:50 GMT, Kevin Rushforth wrote: > The JavaFX SDK bundle provides a set of modular jars in the `lib` directory. > It provides a `src.zip` file for use by IDEs in that same `lib` directory. If > a developer adds the `lib` directory to their application's module path in

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel

2021-07-01 Thread Marius Hanl
On Thu, 1 Jul 2021 14:52:28 GMT, Jeanette Winzenburg wrote: >> This PR fixes 2 NPEs in Choice-and ComboBox, when the selection model is >> null. >> >> ChoiceBox: >> - Null check in **valueProperty()** listener >> >> ComboBox: >> - Null check in **valueProperty()** listener >> - Null check

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel

2021-07-01 Thread Marius Hanl
On Wed, 30 Jun 2021 15:03:50 GMT, Marius Hanl wrote: > This PR fixes 2 NPEs in Choice-and ComboBox, when the selection model is null. > > ChoiceBox: > - Null check in **valueProperty()** listener > > ComboBox: > - Null check in **valueProperty()** li

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel

2021-07-01 Thread Marius Hanl
On Thu, 1 Jul 2021 15:29:01 GMT, Marius Hanl wrote: > I don't see any benefit with a noop selection model, but we need/should > cleanup some null checks and somehow still silently set a selection model > while we as a developer set it to null (so I guess we expected null to be > r

Re: RFR: 8268642: Expand the javafx.beans.property.Property documentation [v2]

2021-06-28 Thread Marius Hanl
On Thu, 24 Jun 2021 01:53:53 GMT, Michael Strauß wrote: >> * Expand the `Property.bind` and `Property.bindBidirectional` documentation >> * Change the name of the formal parameter of `Property.bind` to "source" >> (currently, it is inconsistently named "observable", "rawObservable" or >>

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: 8269244: [IDE] Dependency verification of *-sources.jar fails when doing gradle sync

2021-06-28 Thread Marius Hanl
This PR fixes the issue identified and discussed in PR https://github.com/openjdk/jfx/pull/506 which will make the gradle sync in IntelliJ fail because of the failing dependency verification for source files. Gradle provides a way to skip the verification of source files, documented here:

Integrated: 8269244: [IDE] Dependency verification of *-sources.jar fails when doing gradle sync

2021-06-28 Thread Marius Hanl
On Mon, 28 Jun 2021 14:38:32 GMT, Marius Hanl wrote: > This PR fixes the issue identified and discussed in PR > https://github.com/openjdk/jfx/pull/506 which will make the gradle sync in > IntelliJ fail because of the failing dependency verification for source files. > > Gradle

Re: RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v10]

2021-06-27 Thread Marius Hanl
On Sun, 27 Jun 2021 12:09:24 GMT, Michael Strauß wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java >> line 109: >> >>> 107: >>> 108: @Override public int getRemovedSize() { >>> 109: checkState(); >> >> Is this needed here? Just

Re: RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v10]

2021-06-27 Thread Marius Hanl
On Fri, 25 Jun 2021 13:08:36 GMT, Michael Strauß wrote: >> The documentation for `ObservableListBase.nextRemove` states that a single >> change always refers to the current state of the list, which likely means >> that multiple disjoint removed ranges need to be applied in order, otherwise >>

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v3]

2021-07-08 Thread Marius Hanl
On Thu, 8 Jul 2021 11:59:05 GMT, Jeanette Winzenburg wrote: >>> Hmm ... wondering whether we really want to widen the scope of this issue >>> >>> * it started with being focused on NPE on the change of property value, >>> for both Choice/ComboBox >>> >>> * turned out combo's skin

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v4]

2021-07-11 Thread Marius Hanl
On Fri, 9 Jul 2021 09:58:38 GMT, Jeanette Winzenburg wrote: >> Hmm, but leaving a test without an assert is also bad. You have any >> suggestions? >> I may can add another editable test, which will pass before and after. > >> >> >> Hmm, but leaving a test without an assert is also bad. You

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit

2021-07-11 Thread Marius Hanl
On Fri, 9 Jul 2021 10:51:16 GMT, Jeanette Winzenburg wrote: > > Do we wanna create follow-ups for this, so this PR won''t get any bigger? > > That would be at least my preference. > > Right now I see: > > ``` > > * !isEditing check in cancelEdit() (we can return directly) > > > > * isEditing

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v2]

2021-07-06 Thread Marius Hanl
On Mon, 5 Jul 2021 13:06:48 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - removed now unused imports >> - Review adjustments > > modules/javaf

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v3]

2021-07-06 Thread Marius Hanl
The tests checks, that no NPE is printed to the console. They also checks, > that the set value is still displayed (either in the ComboBox button cell or > the ChoiceBox display label) Marius Hanl has updated the pull request incrementally with one additional commit since the last

RFR: 8231644: TreeTableView Regression: Indentation wrong using Label as column content type

2021-07-06 Thread Marius Hanl
This PR fixes a long standing issue with the TreeTableView indentation. ![image](https://user-images.githubusercontent.com/66004280/124681647-473e7380-dec9-11eb-906d-4228fc39cbf9.png) In short: **TreeTableCellSkin** overrides **leftLabelPadding()** to calculate the indentation (the result of

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v3]

2021-07-08 Thread Marius Hanl
all 4 CheckBox...Cells have a proper CheckBox > disablement while also being nullsafe. > > Note 1: I removed the tests in TableCellTest added in > https://github.com/openjdk/jfx/pull/529 in favor of the new parameterized > TableCellStartEditTest > Note 2: This also fixes https://

Re: RFR: 8231644: TreeTableView Regression: Indentation wrong using Label as column content type [v2]

2021-07-08 Thread Marius Hanl
do it > sometimes via **leftLabelPadding()**. (which is called directly by > **layoutChildren()**) > > Note: I also added some tests which pass before and pass after. Marius Hanl has updated the pull request incrementally with one additional commit since the last revision: calcul

Re: RFR: 8231644: TreeTableView Regression: Indentation wrong using Label as column content type [v2]

2021-07-08 Thread Marius Hanl
On Thu, 8 Jul 2021 21:24:28 GMT, Marius Hanl wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableCellSkin.java >> line 107: >> >>> 105: >>> 106: @Override >>> 107: protected void layoutChildre

Re: RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v4]

2021-07-08 Thread Marius Hanl
On Thu, 8 Jul 2021 10:52:06 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Fixed NPE for setEditable() and layoutChildren() >> - removed unneeded b

Re: RFR: 8231644: TreeTableView Regression: Indentation wrong using Label as column content type

2021-07-08 Thread Marius Hanl
On Wed, 7 Jul 2021 23:57:31 GMT, Kevin Rushforth wrote: >> This PR fixes a long standing issue with the TreeTableView indentation. >> >> ![image](https://user-images.githubusercontent.com/66004280/124681647-473e7380-dec9-11eb-906d-4228fc39cbf9.png) >> >> In short: >> **TreeTableCellSkin**

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit

2021-07-08 Thread Marius Hanl
On Wed, 7 Jul 2021 22:33:07 GMT, Marius Hanl wrote: > This PR sets an unified logic to every **startEdit()** method of all Cell > implementations. > So startEdit() is always doing the same now: > > `super.startEdit();` > `if (!isEditing()) { > return; > }` > >

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v2]

2021-07-08 Thread Marius Hanl
all 4 CheckBox...Cells have a proper CheckBox > disablement while also being nullsafe. > > Note 1: I removed the tests in TableCellTest added in > https://github.com/openjdk/jfx/pull/529 in favor of the new parameterized > TableCellStartEditTest > Note 2: This also fixes https://

RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel

2021-06-30 Thread Marius Hanl
This PR fixes 2 NPEs in Choice-and ComboBox, when the selection model is null. ChoiceBox: - Null check in **valueProperty()** listener ComboBox: - Null check in **valueProperty()** listener - Null check in **ComboBoxListViewSkin#updateValue()** The tests checks, that no NPE is printed to the

Re: RFR: 8258499: JavaFX: Move src.zip out of the lib directory

2021-06-30 Thread Marius Hanl
On Wed, 30 Jun 2021 15:06:50 GMT, Kevin Rushforth wrote: > The JavaFX SDK bundle provides a set of modular jars in the `lib` directory. > It provides a `src.zip` file for use by IDEs in that same `lib` directory. If > a developer adds the `lib` directory to their application's module path in

Re: RFR: 8258499: JavaFX: Move src.zip out of the lib directory

2021-06-30 Thread Marius Hanl
On Wed, 30 Jun 2021 16:16:16 GMT, Kevin Rushforth wrote: > Thanks for running the test. This is sufficient to show that the changed > layout will work with IntelliJ. > > What happens with the current layout, where src.zip is in the lib dir, if you > add the lib dir? Does IntelliJ fail in the

Re: RFR: 8187229: Tree/TableCell: cancel event must return correct editing location [v2]

2021-07-06 Thread Marius Hanl
On Mon, 5 Jul 2021 10:18:15 GMT, Jeanette Winzenburg wrote: >> the bug is an incorrect edit location (for Tree/Table: Tree/TablePosition) >> in edit cancel events - expected is the location at the time the cell edit >> was started, actual was the location of at the time the edit was

RFR: 8267392: ENTER key press on editable TableView throws NPE

2021-05-19 Thread Marius Hanl
**Note: I reported the bug already, waiting for approval. Internal tracking id: 9070318. I will update the title as soon as the ticket is created.** This PR is fixing a NP, which is thrown when you press ENTER on an editbale table, after it is initially shown. When pressing ENTER,

Re: RFR: 8267392: ENTER key press on editable TableView throws NPE

2021-05-19 Thread Marius Hanl
On Wed, 19 May 2021 11:03:52 GMT, Jeanette Winzenburg wrote: > > > sounds like a duplicate of > [JDK-8089652](https://bugs.openjdk.java.net/browse/JDK-8089652) - faintly > remember that it's a dark pit: there are issues in selection/focus model > implementations, see f.i. >

Re: RFR: 8267392: ENTER key press on editable TableView throws NPE [v2]

2021-05-19 Thread Marius Hanl
> items from the underlying table are changed (e.g. **setItems()**) or when > **getFocusModel().focus(row)** is used. > Therefore, null is a valid value and we should guard against it. Marius Hanl has updated the pull request incrementally with one additional commit since the last revision:

Re: RFR: 8267392: ENTER key press on editable TableView throws NPE [v2]

2021-05-19 Thread Marius Hanl
On Wed, 19 May 2021 12:15:55 GMT, Kevin Rushforth wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Addressed review comment (changed from NP to NPE) > > modules/javafx.controls/s

RFR: 8267418: IntelliJ build and test of JavaFX does not work

2021-05-19 Thread Marius Hanl
Question: I was wondering, should I create a ticket for this as well? Given the fact that I don't have an https://bugs.openjdk.java.net account, I need to use the official bug reporting tool, which looked a bit overkill to me since someone needs to check my created ticket, while this PR is only

Re: Re: Convenience factories for Border and Background

2021-04-26 Thread Marius Hanl
I'm fine with either way. Both approaches are nice, while the factory/builder approach will probably be a bit better as it will cover more use cases, but of course takes a bit more time. In any case we can create a Border/Background very easily and both are quite short, the

RFR: 8186904: TableColumnHeader: resize cursor lost on right click

2021-04-28 Thread Marius Hanl
This PR is fixing a bug, when a right click on a table column is performed. By doing that, the table columns will lose the resize cursor thus they can not be resized anymore. The reason for that is that the **columnDragLock** will not reset (to false). This flag is set to true, when a mouse

Re: RFR: 8231601: Update CONTRIBUTING.md to clarify process for contributing features plus Skara changes [v3]

2021-04-29 Thread Marius Hanl
On Fri, 18 Sep 2020 18:57:16 GMT, Kevin Rushforth wrote: >> This PR updates the `CONTRIBUTING.md` guide to address the following: >> >> 1. Clarify the process for adding new features / API changes, specifically >> that they must be discussed on the mailing list as the first step. >> 2. Add a

RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed

2021-03-28 Thread Marius Hanl
This PR fixes an issue, where table cells are not removed from the table row when the corresponding table column got removed. This will lead to empty "ghost" cells laying around in the table. This bug only occurs, when a fixed cell size is set to the table. I also added 3 more tests beside the

Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed [v2]

2021-03-30 Thread Marius Hanl
(false) of a column should both do the > same: Remove the corresponding cells, so that there are no empty cells. > Therefore, the additional tests make sure, that the other use cases (still) > works and won't break in future (at least, without red tests ;)). > See also: TableRowSkinB

Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed [v2]

2021-03-30 Thread Marius Hanl
On Tue, 30 Mar 2021 13:27:21 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8258663: Using VirtualFlowTestUtils in tests now instead of own solution >&g

Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed

2021-03-29 Thread Marius Hanl
On Mon, 29 Mar 2021 11:40:40 GMT, Jeanette Winzenburg wrote: >> This PR fixes an issue, where table cells are not removed from the table row >> when the corresponding table column got removed. This will lead to empty >> "ghost" cells laying around in the table. >> This bug only occurs, when a

Re: RFR: 8264330: Scene MouseHandler is referencing removed nodes

2021-03-31 Thread Marius Hanl
On Wed, 31 Mar 2021 05:36:06 GMT, John Hendrikx wrote: > Small fix to clear a reference to a removed node left by Scene$MouseHandler. modules/javafx.graphics/src/test/java/test/javafx/scene/SceneTest.java line 978: > 976: > 977: @Test public void >

Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed [v2]

2021-03-31 Thread Marius Hanl
On Wed, 31 Mar 2021 12:00:39 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8258663: Using VirtualFlowTestUtils in tests now instead of own solution >&g

Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed [v2]

2021-03-31 Thread Marius Hanl
On Wed, 31 Mar 2021 13:20:09 GMT, Kevin Rushforth wrote: >> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java >> line 42: >> >>> 40: >>> 41: import static junit.framework.Assert.assertEquals; >>> 42: >> >> shouldn't that be `org.junit.Assert.*` ? > >

Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed [v2]

2021-03-31 Thread Marius Hanl
On Wed, 31 Mar 2021 12:32:02 GMT, Jeanette Winzenburg wrote: > > > Fix looks good, verified failing/passing test before/after fix. Left minor > comments inline. > > As to the test - good to increase test coverage by including tests for > invisible columns, IMO :) > > Two thingies you

Re: RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed [v3]

2021-03-31 Thread Marius Hanl
(false) of a column should both do the > same: Remove the corresponding cells, so that there are no empty cells. > Therefore, the additional tests make sure, that the other use cases (still) > works and won't break in future (at least, without red tests ;)). > See also: TableRowSkinB

Re: RFR: 8252936: Optimize removal of listeners from ExpressionHelper.Generic

2021-04-01 Thread Marius Hanl
On Thu, 10 Sep 2020 15:49:59 GMT, yosbits wrote: >> @yososs Please file a new JBS issue for this. You will need to prove that >> your proposed change is functionally equivalent (or that any perceived >> changes are incidental and still conform to the spec). You should also think >> about

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button

2021-04-06 Thread Marius Hanl
On Mon, 5 Apr 2021 23:01:25 GMT, Kevin Rushforth wrote: > > > I have one question on the fix (see below). Also, have you run all of the > unit tests (not just the new one)? I did run all the normal tests and ever dialog related system test. Everything passed. Isn't jcheck running all the

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button

2021-04-06 Thread Marius Hanl
On Mon, 5 Apr 2021 22:58:47 GMT, Kevin Rushforth wrote: >> When DialogPane#getButtonTypes().setAll() is called twice with the same >> argument(s), DialogPane#lookupButton does not return the node which is shown >> inside the button bar. >> This is due DialogPane adding two list change

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button [v2]

2021-04-06 Thread Marius Hanl
On Tue, 6 Apr 2021 12:06:05 GMT, Kevin Rushforth wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8263807: Code review changes > > modules/javafx.controls/src/test/java/test/javafx/sce

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button [v2]

2021-04-06 Thread Marius Hanl
the wrong order, which will result in the button > bar not changing at all and the 'buttonNodes' list will recreate the dialog > button(s). > Finally, this will make DialogPane#lookupButton returning the 'wrong' button, > which is in fact not used inside the dialog button bar. Mariu

Re: RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button [v2]

2021-04-06 Thread Marius Hanl
On Tue, 6 Apr 2021 12:18:49 GMT, Kevin Rushforth wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8263807: Code review changes > > The fix and test look fine. There is on

RFR: 8263807: Button types of a DialogPane are set twice, returns a wrong button

2021-03-18 Thread Marius Hanl
When DialogPane#getButtonTypes().setAll() is called twice with the same argument(s), DialogPane#lookupButton does not return the node which is shown inside the button bar. This is due DialogPane adding two list change listeners to 'buttons' (#getButtonTypes). They have the wrong order, which

Integrated: 8258663: Fixed size TableCells are not removed from sene graph when column is removed

2021-04-08 Thread Marius Hanl
On Sun, 28 Mar 2021 23:13:22 GMT, Marius Hanl wrote: > This PR fixes an issue, where table cells are not removed from the table row > when the corresponding table column got removed. This will lead to empty > "ghost" cells laying around in the table. > This bug only occ

Integrated: 8263807: Button types of a DialogPane are set twice, returns a wrong button

2021-04-08 Thread Marius Hanl
On Thu, 18 Mar 2021 14:38:18 GMT, Marius Hanl wrote: > When DialogPane#getButtonTypes().setAll() is called twice with the same > argument(s), DialogPane#lookupButton does not return the node which is shown > inside the button bar. > This is due DialogPane adding two list chan

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v3]

2021-08-17 Thread Marius Hanl
On Tue, 17 Aug 2021 09:26:12 GMT, Jeanette Winzenburg wrote: >> I see, another point where I would love we actually use JUnit5. :/ > > ehh .. this is not resolved - still change required - or what do I miss? I already changed the tests to use a supplier but didn't pushed it yet. I just

Re: Managed Property - CSS Styleable

2021-08-13 Thread Marius Hanl
I also think this is a great addition. Gesendet: Donnerstag, 12. August 2021 um 21:49 Uhr Von: "Abhinay Agarwal" An: "openjfx-dev@openjdk.java.net" Betreff: Managed Property - CSS Styleable Every now and then, I miss having the capability to (un)manage nodes in its Parent

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v4]

2021-08-17 Thread Marius Hanl
all 4 CheckBox...Cells have a proper CheckBox > disablement while also being nullsafe. > > ~Note 1: I removed the tests in TableCellTest added in > https://github.com/openjdk/jfx/pull/529 in favor of the new parameterized > TableCellStartEditTest~ > Note 2: This also

Aw: Re: Convenience factories for Border and Background

2021-08-24 Thread Marius Hanl
I want to see this as well. Do we want to continue creating a draft for this? Am 23.08.21, 01:48 schrieb Nir Lisker : Getting this moving again as well. > Another option could be to mirror the `Color` API in both `Border` and > `Background`, like in the following

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v3]

2021-08-16 Thread Marius Hanl
On Wed, 28 Jul 2021 15:12:45 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Reordered commit > > modules/javafx.controls/src/test/java/test/javafx/scene/cont

Re: RFR: 8244419: TextAreaSkin: throws UnsupportedOperation on dispose

2021-08-16 Thread Marius Hanl
On Sat, 14 Aug 2021 10:32:00 GMT, Jeanette Winzenburg wrote: > The issue was a glaring contract violation of TextAreaSkin which throws a > UnsupportedOperationException. The fix was to remove the throwing and cleanup > on dispose which implies > > in TextAreaBehavior: > - remove the listener

Re: RFR: 8272870: Add convenience factory methods for border and background [v2]

2021-09-03 Thread Marius Hanl
On Fri, 27 Aug 2021 17:40:48 GMT, Nir Lisker wrote: >> Added convenience factory factory methods for Background and Border. > > Nir Lisker has updated the pull request incrementally with one additional > commit since the last revision: > > Corrected comment tags

Re: RFR: 8273324: IllegalArgumentException: fromIndex(0) > toIndex(-1) after clear and select TableCell

2021-09-04 Thread Marius Hanl
On Fri, 3 Sep 2021 21:17:11 GMT, Michael Strauß wrote: > > Can we get a unit test for this case? > > This is a test that fails without the fix, and passes with the fix. However, > there are no assertions, which is... strange. Once >

Integrated: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel

2021-09-09 Thread Marius Hanl
On Wed, 30 Jun 2021 15:03:50 GMT, Marius Hanl wrote: > This PR fixes multiple NPEs in Choice-and ComboBox, when the selection model > is null. > > ChoiceBox: > - Null check in **valueProperty()** listener > > ComboBox: > - Null check in **editableProperty()**

Re: RFR: 8208088: Memory Leak in ControlAcceleratorSupport [v6]

2021-09-10 Thread Marius Hanl
On Fri, 10 Sep 2021 10:30:07 GMT, RationalityFrontline wrote: > After updating to jfx 17, I detected memory leak in my application (every > controller that has menu items won't get garbage collected after closing its > stage), with visualvm I found it was caused by class >

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v4]

2021-09-19 Thread Marius Hanl
On Thu, 19 Aug 2021 12:25:16 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Separated test and made the cell a supplier instead > > modules/javafx.contr

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v5]

2021-09-19 Thread Marius Hanl
all 4 CheckBox...Cells have a proper CheckBox > disablement while also being nullsafe. > > ~Note 1: I removed the tests in TableCellTest added in > https://github.com/openjdk/jfx/pull/529 in favor of the new parameterized > TableCellStartEditTest~ > Note 2: This also

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v4]

2021-09-19 Thread Marius Hanl
On Thu, 19 Aug 2021 12:16:28 GMT, Jeanette Winzenburg wrote: >> Marius Hanl has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Separated test and made the cell a supplier instead > > modules/javafx.contr

Re: RFR: 8272870: Add convenience factory methods for border and background [v3]

2021-09-19 Thread Marius Hanl
On Thu, 16 Sep 2021 21:15:20 GMT, Kevin Rushforth wrote: > > > ``` > > > public static Border stroke(Paint stroke, double width) { > > > ``` > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ... > > > But I really want to hear other

Re: RFR: 8272870: Add convenience factory methods for border and background [v3]

2021-09-16 Thread Marius Hanl
On Thu, 9 Sep 2021 23:43:29 GMT, Nir Lisker wrote: >> Added convenience factory factory methods for Background and Border. > > Nir Lisker has updated the pull request incrementally with two additional > commits since the last revision: > > - Removed whitespaces > - Added tests and doc

Re: RFR: 8188026: TextFieldXXCell: NPE on calling startEdit

2021-09-13 Thread Marius Hanl
On Thu, 22 Jul 2021 19:15:54 GMT, Marius Hanl wrote: >>> > just checked my notes (there's a cell-editing branch in my fork where I'm >>> > experimenting) - astonishingly the answer is no, could not see anything >>> > :) And actually, seems like we d

Re: RFR: 8269871: CellEditEvent: must not throw NPE in accessors

2021-09-03 Thread Marius Hanl
On Thu, 26 Aug 2021 14:09:58 GMT, Jeanette Winzenburg wrote: > The issue is unguarded access to tablePosition though it might be null (since > [JDK-8120610](https://bugs.openjdk.java.net/browse/JDK-8120610) > > The fix is to check against null in each accessor. Also required to fix the >

  1   2   3   >