Re: RFR: 8272779: Package docs for javafx.embed.swing are misleading [v2]

2021-08-23 Thread Prasanta Sadhukhan
> The API docs for the javafx.embed.swing package are misleading in that they 
> only talk about the JFXPanel capability (embedding a JavaFX Scene in a Swing 
> JComponent) and ignore the SwingNode functionality entirely. 
> Fix the package doc.

Prasanta Sadhukhan has updated the pull request incrementally with one 
additional commit since the last revision:

  Fix

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/609/files
  - new: https://git.openjdk.java.net/jfx/pull/609/files/88b9b285..dd60daad

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=609=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=609=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/609.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/609/head:pull/609

PR: https://git.openjdk.java.net/jfx/pull/609


RFR: 8272779: Package docs for javafx.embed.swing are misleading

2021-08-23 Thread Prasanta Sadhukhan
The API docs for the javafx.embed.swing package are misleading in that they 
only talk about the JFXPanel capability (embedding a JavaFX Scene in a Swing 
JComponent) and ignore the SwingNode functionality entirely. 
Fix the package doc.

-

Commit messages:
 - Fix
 - Fix

Changes: https://git.openjdk.java.net/jfx/pull/609/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=609=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8272779
  Stats: 18 lines in 1 file changed: 13 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jfx/pull/609.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/609/head:pull/609

PR: https://git.openjdk.java.net/jfx/pull/609


Re: [External] : Re: Pivot properties [was: Enhancements for JavaFX 18]

2021-08-23 Thread Michael Strauß
I think normalized coordinates are a very natural fit for the
definition of a pivot point, which is why the current default value is
an implicitly-specified normalized coordinate. Adding general-purpose
coordinate transformations here feels like bringing a sledgehammer to
crack a nut.

Having a property with an automagically updated "initial" value is
quite non-standard behavior for properties. It would appear as if the
property was bound, yet there's no binding. That's markedly different
from a Transition's 'from' and 'to' properties, which are standard
properties with normal behavior. A running transition doesn't bind to
those properties, it uses a snapshot of their values when the
transition starts.


On Sun, Aug 22, 2021 at 9:55 PM Nir Lisker  wrote:
>
> Now I understand what you meant. However, the concept of normalized 
> coordinates does not appear anywhere in JavaFX (at least not in these 
> contexts). I still think that coordinate transformations should be handled 
> via dedicated means, like coordinate system transformations are. Maybe when 
> we work on mapped bindings (I forgot that I need to review that) this pain 
> point will be alleviated.
>
> Kevin, what if we only set the initial value of the pivot (doesn't matter 
> what the implementation detail is) to the center of the node for backwards 
> compatibility, but if the developer changes it to a custom value then it's on 
> them to compute the value again if they want to go back? The default behavior 
> remains.
> Another relevant point is that Transitions do something similar with their 
> from_,  to_ and by_ methods. They start with Double.NaN to signal that the 
> value should be ignored. While the developer can reset the value to NaN, it 
> is not something that is likely to happen during, say, an interpolation or as 
> a result of a binding.
>


Re: RFR: 8169098: [TestBug] Manual test case for JDK-8089915 [v2]

2021-08-23 Thread Kevin Rushforth
On Mon, 23 Aug 2021 14:27:08 GMT, Rahul Kumar 
 wrote:

>> Adding a new manual test case for accept attribute of input tag.
>> This test is to verify the fix for 
>> [JDK-8089915](https://bugs.openjdk.java.net/browse/JDK-8089915) .
>> We have tested the test cases on all three platforms and its working as 
>> expected.
>
> Rahul Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Incorporate review comment

Looks good.

-

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/607


Re: Convenience factories for Border and Background

2021-08-23 Thread Duncan Mak
It feels more clear to me to see:Border.stroke(Paint stroke) and
Background.fill(Paint fill)

I vote in favor of those instead of Border.or and Background.of.

On Sun, Aug 22, 2021 at 7:46 PM Nir Lisker  wrote:

> Getting this moving again as well.
>
>
> > Another option could be to mirror the `Color` API in both `Border` and
> > `Background`, like in the following examples:
>
>
> Color.rgb(125, 100, 75)
> > Border.rgb(125, 100, 75)
> > Background.rgb(125, 100, 75)
>
>
> Color.gray(127)
> > Border.gray(127)
> > Background.gray(127)
>
>
> > Color.web("orange", 0.5)
> > Border.web("orange", 0.5)
> > Background.web("orange", 0.5)
>
>
> This is possible, but I don't think it saves much. This API in Color makes
> it easy to create a color, so just using that directly in the
> border/background seems convenient enough to me. Comparing
> Border.rgb(125, 100, 75);
> with
> Border.of(Color.rgb(125, 100, 75)); // whatever the method name ends up
> being
> tells me that funneling all the color creation ways into one method in
> border/background is efficient enough to not merit multiple methods.
>
> We could also mirror the named color constants, which would enable a
> > very compact syntax:
>
>
>
> > StackPane pane = new StackPane();
> > pane.setBorder(Border.RED);
> > pane.setBackground(Background.BLUE);
>
>
> This is very similar to how "red" or "blue" are valid values for
> > "-fx-border" or "-fx-background" in CSS.
>
>
> I rather not duplicate hundreds of constants just to be able to do
> pane.setBorder(Border.RED);
> instead of
> pane.setBorder(Border.of(Color.RED));
>
> On Tue, Jun 8, 2021 at 2:41 AM Nir Lisker  wrote:
>
> > Does this constitute sufficient interest in the enhancement?
> >
> > On Thu, May 13, 2021 at 6:41 PM Michael Strauß 
> > wrote:
> >
> >> Another option could be to mirror the `Color` API in both `Border` and
> >> `Background`, like in the following examples:
> >>
> >> Color.rgb(125, 100, 75)
> >> Border.rgb(125, 100, 75)
> >> Background.rgb(125, 100, 75)
> >>
> >> Color.gray(127)
> >> Border.gray(127)
> >> Background.gray(127)
> >>
> >> Color.web("orange", 0.5)
> >> Border.web("orange", 0.5)
> >> Background.web("orange", 0.5)
> >>
> >> We could also mirror the named color constants, which would enable a
> >> very compact syntax:
> >>
> >> StackPane pane = new StackPane();
> >> pane.setBorder(Border.RED);
> >> pane.setBackground(Background.BLUE);
> >>
> >> This is very similar to how "red" or "blue" are valid values for
> >> "-fx-border" or "-fx-background" in CSS.
> >>
> >
>


-- 
Duncan.


Re: RFR: 8267472: JavaFX modules to include version information [v3]

2021-08-23 Thread Christian Stein
On Mon, 23 Aug 2021 18:18:51 GMT, Christian Stein  wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8267472
>
> Christian Stein has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add module version to `graphics` and `web` project's compiler args

Perhaps something for JFX 18-ea... 

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v3]

2021-08-23 Thread Christian Stein
On Mon, 23 Aug 2021 18:40:55 GMT, Johan Vos  wrote:

>> OK, that seems reasonable, and I can't think of any downside.
>
> The only thing that makes me slightly nervous is seeing that this is needed 
> on a number of places, leading to more code duplication (and the build.gradle 
> is already rather large). But that is not a problem introduced with this PR, 
> so I'm not requesting a change for this.

Yeah. At least, the four duplicated places look very similar to each other. 
Perhaps, those lines (adding module-related compiler args) can be extracted 
into a dedicated method. But that should be done, if at all, in a different 
task/PR.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v3]

2021-08-23 Thread Kevin Rushforth
On Mon, 23 Aug 2021 18:18:51 GMT, Christian Stein  wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8267472
>
> Christian Stein has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add module version to `graphics` and `web` project's compiler args

No, we don't have such a check.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v3]

2021-08-23 Thread Johan Vos
On Mon, 23 Aug 2021 16:43:34 GMT, Kevin Rushforth  wrote:

>> Seeing version information in stack traces of failed tests can be helpful. 
>> ;-)
>
> OK, that seems reasonable, and I can't think of any downside.

The only thing that makes me slightly nervous is seeing that this is needed on 
a number of places, leading to more code duplication (and the build.gradle is 
already rather large). But that is not a problem introduced with this PR, so 
I'm not requesting a change for this.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v3]

2021-08-23 Thread Christian Stein
On Mon, 23 Aug 2021 18:18:51 GMT, Christian Stein  wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8267472
>
> Christian Stein has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add module version to `graphics` and `web` project's compiler args

Added version information to both places.

fwiw, in JUnit 5's integration tests, we verify that all module descriptors of 
published modules match some [Golden 
Files](https://github.com/junit-team/junit5/tree/main/platform-tooling-support-tests/projects/jar-describe-module)
 produced by `jar --describe-module --file ...jar`. This includes a check for 
the version being correctly compiled into and reported by them. Is there 
something like that in place for JavaFX's modular JAR files?

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v3]

2021-08-23 Thread Christian Stein
> https://bugs.openjdk.java.net/browse/JDK-8267472

Christian Stein has updated the pull request incrementally with one additional 
commit since the last revision:

  Add module version to `graphics` and `web` project's compiler args

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/608/files
  - new: https://git.openjdk.java.net/jfx/pull/608/files/e9eabd0d..3fdb277d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=608=02
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=608=01-02

  Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/608.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/608/head:pull/608

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v2]

2021-08-23 Thread Kevin Rushforth
On Mon, 23 Aug 2021 14:21:28 GMT, Christian Stein  wrote:

> Aren't they covered by the allprojects { ... } and

No. These options are used for the `compileJava` task in all projects. They do 
not affect the two compile tasks I mentioned. You can see that if you look at 
the following temporary build file:


modules/javafx.graphics/build/tmp/compileFullJava/java-compiler-args.txt


The same will be true for the `compileJavaDOMBindingTask` task I mentioned, if 
you build WebKit.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v2]

2021-08-23 Thread Kevin Rushforth
On Mon, 23 Aug 2021 14:17:20 GMT, Christian Stein  wrote:

>> build.gradle line 3929:
>> 
>>> 3927: options.compilerArgs.addAll([
>>> 3928: '-implicit:none',
>>> 3929: '--module-version', "$RELEASE_VERSION_SHORT",
>> 
>> I'm not sure this is needed (although it doesn't hurt), since the shims are 
>> only used for testing and are not included in any jar file.
>
> Seeing version information in stack traces of failed tests can be helpful. ;-)

OK, that seems reasonable, and I can't think of any downside.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v2]

2021-08-23 Thread Jose Pereda
On Thu, 12 Aug 2021 21:33:57 GMT, Kevin Rushforth  wrote:

>> Jose Pereda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Set since 18
>
> The API for the new property looks fine. I left a couple comments on the 
> javadoc.
> 
> You can create a Draft CSR when you get a chance. You still need to update 
> `cssref.html` and that will need to be part of the CSR.

@kevinrushforth the CSR was already created here: 
https://bugs.openjdk.java.net/browse/JDK-8269848

Do I need to update its code with the latest changes of this PR?

As for the `cssref.html` update, do I add it as a commit to this PR, or as a 
new PR with the CSR id?

-

PR: https://git.openjdk.java.net/jfx/pull/563


Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v2]

2021-08-23 Thread Jose Pereda
On Thu, 12 Aug 2021 21:28:53 GMT, Kevin Rushforth  wrote:

>> Jose Pereda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Set since 18
>
> modules/javafx.web/src/main/java/javafx/scene/web/WebView.java line 702:
> 
>> 700: /**
>> 701:  * Specifies the background color of the webPage, allowing
>> 702:  * some or full transparency.
> 
> You might want to split this into two sentences, with the part about allowing 
> for transparent being in the second sentence. Another thing you should 
> indicate is how this interacts with the background color in the HTML file (I 
> presume the one in the file takes precedence?). Also "web page" should be two 
> words.

Done

> modules/javafx.web/src/main/java/javafx/scene/web/WebView.java line 704:
> 
>> 702:  * some or full transparency.
>> 703:  *
>> 704:  * Default color: White
> 
> Use an `@defaultValue` javadoc tag here:
> 
> 
> @defaultValue {@code Color.WHITE}

Done

-

PR: https://git.openjdk.java.net/jfx/pull/563


Re: RFR: 8090547: Allow for transparent backgrounds in WebView [v3]

2021-08-23 Thread Jose Pereda
> Currently, `WebPage` has already a public `setBackgroundColor()` method, but 
> the class is not public. Therefore, public API is needed in `WebView` to 
> allow developers access to it.
> 
> In line with the `fontSmoothingType` property, this PR provides public 
> support for setting the background color of a WebPage, by adding a `pageFill` 
> property, and a CSR is required.
> 
> The color for the background, that can be opaque, transparent or with any 
> level of opacity, can be set via code or via CSS using `-fx-page-fill`.
> 
> Unit tests and a system test are provided.

Jose Pereda has updated the pull request incrementally with one additional 
commit since the last revision:

  Address feedback from reviewer

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/563/files
  - new: https://git.openjdk.java.net/jfx/pull/563/files/fcd75f55..9676bb0b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=563=02
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=563=01-02

  Stats: 10 lines in 1 file changed: 7 ins; 1 del; 2 mod
  Patch: https://git.openjdk.java.net/jfx/pull/563.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/563/head:pull/563

PR: https://git.openjdk.java.net/jfx/pull/563


Re: RFR: 8169098: [TestBug] Manual test case for JDK-8089915 [v2]

2021-08-23 Thread Rahul Kumar
> Adding a new manual test case for accept attribute of input tag.
> This test is to verify the fix for 
> [JDK-8089915](https://bugs.openjdk.java.net/browse/JDK-8089915) .
> We have tested the test cases on all three platforms and its working as 
> expected.

Rahul Kumar has updated the pull request incrementally with one additional 
commit since the last revision:

  Incorporate review comment

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/607/files
  - new: https://git.openjdk.java.net/jfx/pull/607/files/3b93c3e1..2cf9661e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=607=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=607=00-01

  Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/607.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/607/head:pull/607

PR: https://git.openjdk.java.net/jfx/pull/607


Re: RFR: 8169098: [TestBug] Manual test case for JDK-8089915 [v2]

2021-08-23 Thread Rahul Kumar
On Thu, 19 Aug 2021 16:06:00 GMT, Kevin Rushforth  wrote:

>> Rahul Kumar has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Incorporate review comment
>
> tests/manual/web/InputTypeAcceptAttributeTest.java line 26:
> 
>> 24:  */
>> 25: 
>> 26: 
> 
> Minor: can you remove the extra blank line?

I have updated the review comments.

> tests/manual/web/InputTypeAcceptAttributeTest.java line 37:
> 
>> 35: import javafx.stage.Stage;
>> 36: import javafx.scene.layout.VBox;
>> 37: import javafx.scene.layout.HBox;
> 
> Suggestion: maybe sort the list of imports?

I have updated the review comments.

-

PR: https://git.openjdk.java.net/jfx/pull/607


Re: RFR: 8267472: JavaFX modules to include version information [v2]

2021-08-23 Thread Christian Stein
On Mon, 23 Aug 2021 14:10:56 GMT, Christian Stein  wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8267472
>
> Christian Stein has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Pass module version also to `jmod`

> I think there may be two other tasks where the --module-version is needed: 
> :graphics:compileFullJava and :web:compileJavaDOMBindingTask.

So far to the "single place where Java module related options are found" 
argument.

Aren't they covered by the `allprojects { ... }` and

...
// If I am a module
if (project.hasProperty('moduleSourcePath') &&
(project.hasProperty('buildModule') && project.buildModule)) { ... 
}`
...
```  
...selector?

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v2]

2021-08-23 Thread Christian Stein
On Mon, 23 Aug 2021 14:06:31 GMT, Kevin Rushforth  wrote:

>> Christian Stein has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Pass module version also to `jmod`
>
> build.gradle line 3929:
> 
>> 3927: options.compilerArgs.addAll([
>> 3928: '-implicit:none',
>> 3929: '--module-version', "$RELEASE_VERSION_SHORT",
> 
> I'm not sure this is needed (although it doesn't hurt), since the shims are 
> only used for testing and are not included in any jar file.

Seeing version information in stack traces of failed tests can be helpful. ;-)

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8169098: [TestBug] Manual test case for JDK-8089915

2021-08-23 Thread Kevin Rushforth
On Wed, 18 Aug 2021 15:58:21 GMT, Rahul Kumar 
 wrote:

> Adding a new manual test case for accept attribute of input tag.
> This test is to verify the fix for 
> [JDK-8089915](https://bugs.openjdk.java.net/browse/JDK-8089915) .
> We have tested the test cases on all three platforms and its working as 
> expected.

The test looks good to me. I left two minor formatting comments.

tests/manual/web/InputTypeAcceptAttributeTest.java line 26:

> 24:  */
> 25: 
> 26: 

Minor: can you remove the extra blank line?

tests/manual/web/InputTypeAcceptAttributeTest.java line 37:

> 35: import javafx.stage.Stage;
> 36: import javafx.scene.layout.VBox;
> 37: import javafx.scene.layout.HBox;

Suggestion: maybe sort the list of imports?

-

PR: https://git.openjdk.java.net/jfx/pull/607


RFR: 8169098: [TestBug] Manual test case for JDK-8089915

2021-08-23 Thread Rahul Kumar
Adding a new manual test case for accept attribute of input tag.
This test is to verify the fix for 
[JDK-8089915](https://bugs.openjdk.java.net/browse/JDK-8089915) .
We have tested the test cases on all three platforms and its working as 
expected.

-

Commit messages:
 - Incorporate review changes
 - whitespace correction
 - JDK-8169098: Manual Test case

Changes: https://git.openjdk.java.net/jfx/pull/607/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=607=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8169098
  Stats: 111 lines in 1 file changed: 111 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/607.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/607/head:pull/607

PR: https://git.openjdk.java.net/jfx/pull/607


Re: RFR: 8267472: JavaFX modules to include version information [v2]

2021-08-23 Thread Kevin Rushforth
On Mon, 23 Aug 2021 14:10:56 GMT, Christian Stein  wrote:

>> https://bugs.openjdk.java.net/browse/JDK-8267472
>
> Christian Stein has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Pass module version also to `jmod`

I think there may be two other tasks where the `--module-version` is needed: 
[:graphics:compileFullJava](https://github.com/openjdk/jfx/blob/e9eabd0dbd305a860b8083daf5229764a290e9ef/build.gradle#L2208)
 and 
[:web:compileJavaDOMBindingTask](https://github.com/openjdk/jfx/blob/e9eabd0dbd305a860b8083daf5229764a290e9ef/build.gradle#L3603).

build.gradle line 3929:

> 3927: options.compilerArgs.addAll([
> 3928: '-implicit:none',
> 3929: '--module-version', "$RELEASE_VERSION_SHORT",

I'm not sure this is needed (although it doesn't hurt), since the shims are 
only used for testing and are not included in any jar file.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information [v2]

2021-08-23 Thread Christian Stein
> https://bugs.openjdk.java.net/browse/JDK-8267472

Christian Stein has updated the pull request incrementally with one additional 
commit since the last revision:

  Pass module version also to `jmod`

-

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/608/files
  - new: https://git.openjdk.java.net/jfx/pull/608/files/5d2262ac..e9eabd0d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx=608=01
 - incr: https://webrevs.openjdk.java.net/?repo=jfx=608=00-01

  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/608.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/608/head:pull/608

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Christian Stein
On Mon, 23 Aug 2021 10:48:23 GMT, Christian Stein  wrote:

> https://bugs.openjdk.java.net/browse/JDK-8267472

The submit workflow just started: 
https://github.com/sormuras/jfx/actions/runs/1158948963

I'm not sure, if passing `--module-version` to `jmod` is needed (as the 
`module-info.class` should already contain the compiled version information) -- 
but a) it triggered the workflow and b) it shouln't hurt to set the same 
version twice.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Christian Stein
On Mon, 23 Aug 2021 12:59:46 GMT, Joeri Sykora  wrote:

> [...] I'm not sure if this is more elegant? With Gradle 7, this option is 
> also no longer incubating.

As long as `--module-source-path` et al aren't supported by Gradle, I'd suggest 
to keep Java module related options at a single place.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Kevin Rushforth
On Mon, 23 Aug 2021 13:57:36 GMT, Christian Stein  wrote:

> Enabled actions on my fork - do I need to do something to trigger them now 
> manually?

You need to push a (possibly empty) commit to your branch.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Christian Stein
On Mon, 23 Aug 2021 10:48:23 GMT, Christian Stein  wrote:

> https://bugs.openjdk.java.net/browse/JDK-8267472

Enabled actions on my fork - do I need to do something to trigger them now 
manually? They don't seem to start: 
https://github.com/sormuras/jfx/actions/workflows/submit.yml

-

PR: https://git.openjdk.java.net/jfx/pull/608


[jfx11u] Integrated: 8263760: Update gradle to version 7.0.1

2021-08-23 Thread Ambarish Rapte
On Fri, 20 Aug 2021 20:13:18 GMT, Ambarish Rapte  wrote:

> There were two minor merge conflicts in `build.gradle` and one conflict in 
> `gradle/wrapper/gradle-wrapper.properties` while cherry picking the change. 
> And it also required similar changes to be made for project fxpackager. The 
> [first 
> commit](https://github.com/openjdk/jfx11u/pull/49/commits/7700a6d1a7c82df8fc269f1de6f3bd5119fb3397)
>  is the original cherry pick and [second 
> commit](https://github.com/openjdk/jfx11u/pull/49/commits/cea09bdfc96374ce00560c13e6730ab76f8b20e9)
>  is the additional changes for fxpackager.
> Verified the build on all three platforms with gradle 7.0.1 and apache ant 
> 1.10.5

This pull request has now been integrated.

Changeset: 80c06708
Author:Ambarish Rapte 
URL:   
https://git.openjdk.java.net/jfx11u/commit/80c06708932bc28b7d76cd40cad39c50422788bf
Stats: 148 lines in 13 files changed: 46 ins; 5 del; 97 mod

8263760: Update gradle to version 7.0.1
8240336: JavaFX build uses deprecated features that will be removed in gradle 7
8262236: Configure Gradle checksum verification

Reviewed-by: kcr, jvos
Backport-of: 111bac4180a646662a81223bdbb56880789d5a90

-

PR: https://git.openjdk.java.net/jfx11u/pull/49


Re: RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Kevin Rushforth
On Mon, 23 Aug 2021 10:48:23 GMT, Christian Stein  wrote:

> https://bugs.openjdk.java.net/browse/JDK-8267472

@sormuras Can you enable GitHub actions runs for your personal fork of `jfx`? 
That way we can see the test results as part of the review of this PR. See 
[Checks](https://github.com/openjdk/jfx/pull/608/checks) for more info.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Joeri Sykora
On Mon, 23 Aug 2021 10:48:23 GMT, Christian Stein  wrote:

> https://bugs.openjdk.java.net/browse/JDK-8267472

Looks good to me. Only, since gradle 6.4 (see 
https://github.com/gradle/gradle/issues/2177), you can directly set the module 
version like this:

project.compileJava {
...
options.javaModuleVersion.set("$RELEASE_VERSION_SHORT")
}

I'm not sure if this is more elegant? With Gradle 7, this option is also no 
longer incubating.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: [jfx11u] RFR: 8263760: Update gradle to version 7.0.1 [v2]

2021-08-23 Thread Kevin Rushforth
On Sun, 22 Aug 2021 11:32:58 GMT, Ambarish Rapte  wrote:

>> There were two minor merge conflicts in `build.gradle` and one conflict in 
>> `gradle/wrapper/gradle-wrapper.properties` while cherry picking the change. 
>> And it also required similar changes to be made for project fxpackager. The 
>> [first 
>> commit](https://github.com/openjdk/jfx11u/pull/49/commits/7700a6d1a7c82df8fc269f1de6f3bd5119fb3397)
>>  is the original cherry pick and [second 
>> commit](https://github.com/openjdk/jfx11u/pull/49/commits/cea09bdfc96374ce00560c13e6730ab76f8b20e9)
>>  is the additional changes for fxpackager.
>> Verified the build on all three platforms with gradle 7.0.1 and apache ant 
>> 1.10.5
>
> Ambarish Rapte has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   use antpluginImplementation

Thanks.

> We might remove the fxpackager part, but it doesn't hurt being there either.

Since they aren't being built (and might no longer be buildable), we could 
consider backporting 
[JDK-8203379](https://bugs.openjdk.java.net/browse/JDK-8203379) to jfx11u 
remove the sources in a future update release. As it is, these sources serve no 
purpose other than as a source of "technical debt".

-

PR: https://git.openjdk.java.net/jfx11u/pull/49


Re: [jfx11u] RFR: 8263760: Update gradle to version 7.0.1 [v2]

2021-08-23 Thread Johan Vos
On Sun, 22 Aug 2021 11:32:58 GMT, Ambarish Rapte  wrote:

>> There were two minor merge conflicts in `build.gradle` and one conflict in 
>> `gradle/wrapper/gradle-wrapper.properties` while cherry picking the change. 
>> And it also required similar changes to be made for project fxpackager. The 
>> [first 
>> commit](https://github.com/openjdk/jfx11u/pull/49/commits/7700a6d1a7c82df8fc269f1de6f3bd5119fb3397)
>>  is the original cherry pick and [second 
>> commit](https://github.com/openjdk/jfx11u/pull/49/commits/cea09bdfc96374ce00560c13e6730ab76f8b20e9)
>>  is the additional changes for fxpackager.
>> Verified the build on all three platforms with gradle 7.0.1 and apache ant 
>> 1.10.5
>
> Ambarish Rapte has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   use antpluginImplementation

Marked as reviewed by jvos (Reviewer).

Already did, but forgot to look at the results. Looks good to me. We might 
remove the fxpackager part, but it doesn't hurt being there either.

-

PR: https://git.openjdk.java.net/jfx11u/pull/49


Re: [jfx11u] RFR: 8263760: Update gradle to version 7.0.1 [v2]

2021-08-23 Thread Kevin Rushforth
On Sun, 22 Aug 2021 11:32:58 GMT, Ambarish Rapte  wrote:

>> There were two minor merge conflicts in `build.gradle` and one conflict in 
>> `gradle/wrapper/gradle-wrapper.properties` while cherry picking the change. 
>> And it also required similar changes to be made for project fxpackager. The 
>> [first 
>> commit](https://github.com/openjdk/jfx11u/pull/49/commits/7700a6d1a7c82df8fc269f1de6f3bd5119fb3397)
>>  is the original cherry pick and [second 
>> commit](https://github.com/openjdk/jfx11u/pull/49/commits/cea09bdfc96374ce00560c13e6730ab76f8b20e9)
>>  is the additional changes for fxpackager.
>> Verified the build on all three platforms with gradle 7.0.1 and apache ant 
>> 1.10.5
>
> Ambarish Rapte has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   use antpluginImplementation

@johanvos can you also review this?

-

PR: https://git.openjdk.java.net/jfx11u/pull/49


Integrated: 8244419: TextAreaSkin: throws UnsupportedOperation on dispose

2021-08-23 Thread Jeanette Winzenburg
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 to focusProperty in dispose
> 
> in TextAreaSkin:
> - register all listeners to control properties via skin api
> - remove installed event filter in dispose
> - remove direct children (here only the scrollPane)
> 
> Added tests to guard the listener re-wiring (must pass before and after), and 
> tests to expose side-effects on replacing the skin (fail before, pass after)

This pull request has now been integrated.

Changeset: 5e9f6171
Author:Jeanette Winzenburg 
URL:   
https://git.openjdk.java.net/jfx/commit/5e9f6171289ea20e2d700f2422a4eae50287dd41
Stats: 391 lines in 8 files changed: 332 ins; 37 del; 22 mod

8244419: TextAreaSkin: throws UnsupportedOperation on dispose

Reviewed-by: mhanl, arapte

-

PR: https://git.openjdk.java.net/jfx/pull/604


Re: RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Johan Vos
On Mon, 23 Aug 2021 10:48:23 GMT, Christian Stein  wrote:

> https://bugs.openjdk.java.net/browse/JDK-8267472

This looks good. I'm running the build process and will report once finished.

-

PR: https://git.openjdk.java.net/jfx/pull/608


Re: [jfx11u] RFR: 8263760: Update gradle to version 7.0.1 [v2]

2021-08-23 Thread Kevin Rushforth
On Sun, 22 Aug 2021 11:32:58 GMT, Ambarish Rapte  wrote:

>> There were two minor merge conflicts in `build.gradle` and one conflict in 
>> `gradle/wrapper/gradle-wrapper.properties` while cherry picking the change. 
>> And it also required similar changes to be made for project fxpackager. The 
>> [first 
>> commit](https://github.com/openjdk/jfx11u/pull/49/commits/7700a6d1a7c82df8fc269f1de6f3bd5119fb3397)
>>  is the original cherry pick and [second 
>> commit](https://github.com/openjdk/jfx11u/pull/49/commits/cea09bdfc96374ce00560c13e6730ab76f8b20e9)
>>  is the additional changes for fxpackager.
>> Verified the build on all three platforms with gradle 7.0.1 and apache ant 
>> 1.10.5
>
> Ambarish Rapte has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   use antpluginImplementation

Marked as reviewed by kcr (Lead).

-

PR: https://git.openjdk.java.net/jfx11u/pull/49


Re: RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Kevin Rushforth
On Mon, 23 Aug 2021 10:48:23 GMT, Christian Stein  wrote:

> https://bugs.openjdk.java.net/browse/JDK-8267472

Even though this is a straightforward change, I'd like two reviewers.

-

PR: https://git.openjdk.java.net/jfx/pull/608


RFR: 8267472: JavaFX modules to include version information

2021-08-23 Thread Christian Stein
https://bugs.openjdk.java.net/browse/JDK-8267472

-

Commit messages:
 - Compile version information into modules

Changes: https://git.openjdk.java.net/jfx/pull/608/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jfx=608=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267472
  Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/608.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/608/head:pull/608

PR: https://git.openjdk.java.net/jfx/pull/608


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

2021-08-23 Thread Ambarish Rapte
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 to focusProperty in dispose
> 
> in TextAreaSkin:
> - register all listeners to control properties via skin api
> - remove installed event filter in dispose
> - remove direct children (here only the scrollPane)
> 
> Added tests to guard the listener re-wiring (must pass before and after), and 
> tests to expose side-effects on replacing the skin (fail before, pass after)

Looks good to me too.

-

Marked as reviewed by arapte (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/604