On Tue, 27 Jul 2021 12:41:00 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()** listener
>> - Null check
On Wed, 8 Sep 2021 12:20:57 GMT, Kevin Rushforth wrote:
> > The latest patch does not apply cleanly due to the change in copyright
> > headers.
>
> @aghaisas If you fetch the PR branch and merge master (which is what I
> usually do when testing a PR), git is able to auto-resolve it; this is wh
On Wed, 8 Sep 2021 06:32:06 GMT, Ajit Ghaisas wrote:
> The latest patch does not apply cleanly due to the change in copyright
> headers.
@aghaisas If you fetch the PR branch and merge master (which is what I usually
do when testing a PR), git is able to auto-resolve it; this is why Skara didn'
On Tue, 27 Jul 2021 12:41:00 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()** listener
>> - Null check
On Tue, 27 Jul 2021 12:36:52 GMT, Marius Hanl wrote:
> But I'm wondering, this might require more as we don't set this dirty flag
> anymore as soon as we install a selection model again.
> But thinking about this even more, this would be a general problem as soon as
> we switch the combobox sel
On Tue, 27 Jul 2021 12:41:00 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()** listener
>> - Null check
> 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()** listener
> - Null check in **valueProperty()** listener
> - Null check in **ComboBoxList
On Tue, 27 Jul 2021 09:55:47 GMT, Jeanette Winzenburg
wrote:
> ComboBox comboBox = new ComboBox<>(items);
> comboBox.setSelectionModel(null);
> installDefaultSkin(comboBox);
Ahh yes, I see, nice catch. I will fix it and update this PR.
But I'm wondering, this might require more as we don't
On Thu, 22 Jul 2021 19:49:44 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()** listener
>> - Null check
On Fri, 9 Jul 2021 09:46:04 GMT, Jeanette Winzenburg
wrote:
>
> // another test, exposing one of the NPEs in createList
> ComboBox comboBox = new ComboBox<>(items);
> comboBox.setSelectionModel(null);
> installDefaultSkin(comboBox);
>
> the difference is that setup already installs a s
> 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()* listener*
> - Null check in **valueProperty()** listener
> - Null check in **ComboBoxList
On Wed, 7 Jul 2021 19:37:23 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()* listener*
>> - Null check i
On Thu, 8 Jul 2021 21:05:51 GMT, Marius Hanl wrote:
>> modules/javafx.controls/src/test/java/test/javafx/scene/control/ComboBoxTest.java
>> line 324:
>>
>>> 322:
>>> 323: comboBox.layout();
>>> 324:
>>
>> KISS again :)
>>
>> - buttonCell is unrelated
>> - setting the value is a not
On Sun, 11 Jul 2021 18:28:24 GMT, Marius Hanl wrote:
>>>
>>>
>>> Hmm, but leaving a test without an assert is also bad. You have any
>>> suggestions?
>>
>> Not aware of such a rule - if we fix code throwing an exception there is not
>> much to assert, except that it fails before and passes a
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 ha
On Thu, 8 Jul 2021 21:04:44 GMT, Marius Hanl wrote:
>
>
> Hmm, but leaving a test without an assert is also bad. You have any
> suggestions?
Not aware of such a rule - if we fix code throwing an exception there is not
much to assert, except that it fails before and passes after. And paddling
On Wed, 7 Jul 2021 19:37:23 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()* listener*
>> - Null check i
On Thu, 8 Jul 2021 21:08:56 GMT, Marius Hanl wrote:
>>> > the first two are naturally within the original scope, the third is near
>>> > enough (a property on one of the covered controls) to be included .. the
>>> > last is arguable, IMO - would tend to not include it here but open a
>>> > fol
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 also
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 button cell in test
>
> modules/javafx.
On Thu, 8 Jul 2021 11:19:57 GMT, Ajit Ghaisas wrote:
> > the first two are naturally within the original scope, the third is near
> > enough (a property on one of the covered controls) to be included .. the
> > last is arguable, IMO - would tend to not include it here but open a
> > follow-up
On Thu, 8 Jul 2021 10:28:22 GMT, Jeanette Winzenburg
wrote:
>> hmm ... can't unresolve this (probably because I wasn't involved?) - how to
>> make it part of the review?
>
> hmm .. looks like I can only add comments to a review when being on the
> "files changed" tab, but not on the "Conversat
On Wed, 7 Jul 2021 19:37:23 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()* listener*
>> - Null check i
On Thu, 8 Jul 2021 10:19:54 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 also has a throwing li
On Wed, 7 Jul 2021 19:29:53 GMT, Marius Hanl wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ComboBoxListViewSkin.java
>> line 401:
>>
>>> 399: private void updateValue() {
>>> 400: SingleSelectionModel comboBoxSM =
>>> comboBox.getSelectionModel();
>>> 40
On Thu, 8 Jul 2021 10:18:13 GMT, Jeanette Winzenburg
wrote:
>> added.
>
> 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 also has a thro
On Wed, 7 Jul 2021 19:37:23 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()* listener*
>> - Null check i
> 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
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
>> bubble up instead of try-catch
>
>
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 instead of try-catch
>
> modules/
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 instead of try-catch
>
> modules/
On Tue, 6 Jul 2021 20:20:23 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()** listener
>> - Null check in **ComboB
On Tue, 6 Jul 2021 20:20:23 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()** listener
>> - Null check in **ComboB
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/javafx.controls/src/test/java/test/javafx/scene
> 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
On Sat, 3 Jul 2021 12:58:16 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()** listener
>> - Null check in **ComboB
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: System.setErr(new PrintStream(out, true));
>>> 162
> 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
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()** listener
> - Null check in **ComboBoxListViewS
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
> returned when
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()** listener
> - Null check in **ComboBoxListViewS
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 in
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()** listener
> - Null check in **ComboBoxListViewS
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 co
44 matches
Mail list logo
