Re: RFR: 8272779: Package docs for javafx.embed.swing are misleading [v2]
> 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
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]
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]
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
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]
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]
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]
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]
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]
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]
> 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]
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]
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]
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]
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]
> 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]
> 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]
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]
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]
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
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
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]
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]
> 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
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
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
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
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
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
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
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]
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]
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]
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
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
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]
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
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
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
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