Re: [External] : Re: Improve property system to facilitate correct usage

2021-07-27 Thread Kevin Rushforth
some changes are binary incompatible, they are syntactically transparent. Yes, that's the big problem. Binary compatibility is very important. The value proposition of making the types a bit more clear doesn't justify breaking binary compatibility. -- Kevin On 7/27/2021 6:09 PM,

Re: Improve property system to facilitate correct usage

2021-07-27 Thread Michael Strauß
I should point out that the rest of the JavaFX framework did not require a single code change as a result of the API changes. So while some changes are binary incompatible, they are syntactically transparent. Am Mi., 28. Juli 2021 um 01:39 Uhr schrieb Kevin Rushforth : > > This will take a while

Re: Improve property system to facilitate correct usage

2021-07-27 Thread Kevin Rushforth
This will take a while to work through, and we will need to get general consensus on the API changes. I doubt I can support incompatible breaking changes in this area, given how fundamental property and bindings are to JavaFX. I'll take a look, but it is likely that the incompatible API

Re: Content binding API

2021-07-27 Thread Kevin Rushforth
Looking at content bindings at the same time as regular bindings makes a lot of sense, as long as the proposed changes don't result in the loss of any well-defined, workable functionality (e.g., tightening the restriction that a property P1 that is unidirectionally bound to another property P2

Improve property system to facilitate correct usage

2021-07-27 Thread Michael Strauß
I propose a set of changes to the JavaFX property system that I've outlined in this PR: https://github.com/openjdk/jfx/pull/590 The changes fall into two categories: enforcement of correct usage (there are several cases listed in the PR), and deprecating untyped APIs (for removal in a future

[jfx17] Withdrawn: 8268642: Expand the javafx.beans.property.Property documentation

2021-07-27 Thread Michael Strauß
On Mon, 14 Jun 2021 17:06:34 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: Eclipse: any way to checkout a PR for review?

2021-07-27 Thread Nir Lisker
I'm not really sure what you mean. If you pull from the remote that the PR is on and checkout the remote branch, is it not good enough for a review? On Tue, Jul 27, 2021 at 2:16 PM Jeanette Winzenburg wrote: > > when reviewing a PR with only a few files changed, I simply create a > local branch

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

2021-07-27 Thread Jeanette Winzenburg
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

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

2021-07-27 Thread Jeanette Winzenburg
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

[jfx17] Integrated: 8240640: [macos] Wrong focus behaviour with multiple Alerts

2021-07-27 Thread Pankaj Bansal
On Mon, 19 Jul 2021 19:52:05 GMT, Pankaj Bansal wrote: > The bug is a regression as a result of fix done for JDK-8227366 and is > reproducible on Linux and Mac. This fix is being reverted in this change and > a new bug (JDK-8271054) has been created to redo the JDK-8227366 > > An automated

Re: [jfx17] RFR: 8240640: [macos] Wrong focus behaviour with multiple Alerts [v3]

2021-07-27 Thread Ambarish Rapte
On Fri, 23 Jul 2021 17:40:32 GMT, Pankaj Bansal wrote: >> The bug is a regression as a result of fix done for JDK-8227366 and is >> reproducible on Linux and Mac. This fix is being reverted in this change and >> a new bug (JDK-8271054) has been created to redo the JDK-8227366 >> >> An

Re: [jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation [v3]

2021-07-27 Thread Ajit Ghaisas
> This PR corrects/adds missing documentation for classes in javafx.css package. Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision: 8250590 - add missing @since tag - Changes: - all:

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

2021-07-27 Thread Marius Hanl
> 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

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

2021-07-27 Thread Marius Hanl
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

Re: [jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation [v2]

2021-07-27 Thread Ajit Ghaisas
On Tue, 27 Jul 2021 12:03:02 GMT, Kevin Rushforth wrote: >> This can only be checked manually. >> I have checked and added the `@since` tag to these *Converter nested classes. > > I ran a script to check for missing `@since` tags. The following class are > still missing them: > > >

Re: [jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation [v2]

2021-07-27 Thread Kevin Rushforth
On Tue, 27 Jul 2021 10:00:10 GMT, Ajit Ghaisas wrote: >> modules/javafx.graphics/src/main/java/javafx/css/converter/EffectConverter.java >> line 83: >> >>> 81: /** >>> 82: * Converter to convert DropShadow {@code Effect} >>> 83: * @since 9 >> >> Good catch on the `@since 9` tag.

Re: Eclipse: any way to checkout a PR for review?

2021-07-27 Thread Jeanette Winzenburg
ahh .. yeah, that looks doable - had hoped to get away without installing git and frickle with commandlines ;) thanks Zitat von Kevin Rushforth : I do something like this: git fetch upstream pull/569/head:pr_569 Then you can checkout the "pr_569" branch. I typically will then merge

Re: Eclipse: any way to checkout a PR for review?

2021-07-27 Thread Kevin Rushforth
I do something like this: git fetch upstream pull/569/head:pr_569 Then you can checkout the "pr_569" branch. I typically will then merge in the current upstream/master to test. If you don't have an "upstream" remote you can instead: git fetch https://github.com/openjdk/jfx

Eclipse: any way to checkout a PR for review?

2021-07-27 Thread Jeanette Winzenburg
when reviewing a PR with only a few files changed, I simply create a local branch and c the changes (*cough, pretty sure there's a better way, but then that's the most simple ;). With changes to many files (like f.i. https://github.com/openjdk/jfx/pull/569) that still would be doable,

Re: [jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation [v2]

2021-07-27 Thread Ajit Ghaisas
> This PR corrects/adds missing documentation for classes in javafx.css package. Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision: 8250590 - address review comments - Changes: - all:

Re: [jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation

2021-07-27 Thread Ajit Ghaisas
On Mon, 26 Jul 2021 19:05:51 GMT, Kevin Rushforth wrote: >> This PR corrects/adds missing documentation for classes in javafx.css >> package. > > modules/javafx.graphics/src/main/java/javafx/css/converter/EffectConverter.java > line 83: > >> 81: /** >> 82: * Converter to convert

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

2021-07-27 Thread Jeanette Winzenburg
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

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

2021-07-27 Thread Jeanette Winzenburg
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

Re: [jfx17] RFR: 8250590: Classes and methods in the javafx.css package are missing documentation

2021-07-27 Thread Ajit Ghaisas
On Mon, 26 Jul 2021 18:58:59 GMT, Kevin Rushforth wrote: >> This PR corrects/adds missing documentation for classes in javafx.css >> package. > > modules/javafx.graphics/src/main/java/javafx/css/StyleClass.java line 60: > >> 58: >> 59: /** >> 60: * Returns the index of this

Re: RFR: 8253351: MediaPlayer does not display an mp4 if there no speakers connected to the PC's [v2]

2021-07-27 Thread Johan Vos
On Mon, 26 Jul 2021 22:23:20 GMT, Alexander Matveev wrote: >> Fixed by not failing initialization if DSERR_NODRIVER is returned, which >> will be return if device is not present at all. Fixed format initialization >> even if DirectSound device was not created in case if audio device will >>