8337280: Include jdk.jsobject module with JavaFX

[kevinrushforth]

trafficstars

Draft PR to add the jdk.jsobject module to the JavaFX build.

JDK-8311530 / PR openjdk/jdk#20555 is the JDK PR that proposes to deprecate jdk.jsobject module for removal.

I will need to both build and test this PR with each of the following JDKs:

1. JDK 23 or earlier (e.g., using the default boot JDK), which includes jdk.jsobject as a normal, non-upgradable module
2. A locally-built JDK 24 plus the fix for JDK-8311530, which includes a terminally-deprecated jdk.jsobject as an upgradable module
3. A locally-built JDK 24 plus a patch to locally remove the jdk.jsobject module (a prototype of what will happen in JDK 26 when it is removed)

Based on some very early testing I did, all three modes work (with some limitations) in various modes of testing, such as: using the SDK (both with and without --upgrade-module-path), and using jlink to create a custom JDK + JavaFX. I'll do more rigorous testing using the JDK in PR openjdk/jdk#20555, and with a prototype of the eventual removal.

Once I've finished the testing, I'll take both PRs out of Draft and make them rfr.

---

Progress

• [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue
• [ ] Change requires CSR request JDK-8338249 to be approved

Issues

• JDK-8337280: Include jdk.jsobject module with JavaFX (Enhancement - P4)
• JDK-8338249: Include jdk.jsobject module with JavaFX (CSR)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1529/head:pull/1529
$ git checkout pull/1529

Update a local copy of the PR:
$ git checkout pull/1529
$ git pull https://git.openjdk.org/jfx.git pull/1529/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1529

View PR using the GUI difftool:
$ git pr show -t 1529

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1529.diff

[bridgekeeper[bot]]

:wave: Welcome back kcr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

@kevinrushforth This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8337280: Include jdk.jsobject module with JavaFX

Reviewed-by: arapte, jbhaskar

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[kevinrushforth]

/reviewers 2

[openjdk[bot]]

@kevinrushforth The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

[mlbridge[bot]]

Webrevs

• 06: Full - Incremental (ce91c15c)
• 05: Full - Incremental (4640d7f3)
• 04: Full - Incremental (69513ac2)
• 03: Full - Incremental (6bfc99a9)
• 02: Full - Incremental (2c00deb3)
• 01: Full - Incremental (bb9a3b62)
• 00: Full (182fb1f0)

[AnirvanSarkar]

Old version of classes netscape.javascript.JSObject and netscape.javascript.JSException seems to be delivered with JavaFX for Android [1].

Should these classes be removed?

[1] : https://github.com/openjdk/jfx/tree/jfx23/modules/javafx.web/src/android/java/netscape/javascript

[mlbridge[bot]]

Mailing list message from Johan Vos on openjfx-dev:

Hi Kevin,

Thanks for the extensive checks on these scenario's. I believe the scenario where jlink is used with an "old" JDK (e.g. 21) in combination with the most recent JavaFX release is very common. In that case, it seems to be required to prepend the module path with $JAVA_HOME/jmods, correct? If that is the case, we need to announce and explain it very well, as developers will run into it and get confused.

- Johan

Op do 5 sep 2024 om 23:18 schreef Kevin Rushforth <kcr at openjdk.org>:

-------------- next part -------------- An HTML attachment was scrubbed... URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20240906/56eed1be/attachment-0001.htm>

[kevinrushforth]

Old version of classes netscape.javascript.JSObject and netscape.javascript.JSException seems to be delivered with JavaFX for Android [1].

Should these classes be removed?

[1] : https://github.com/openjdk/jfx/tree/jfx23/modules/javafx.web/src/android/java/netscape/javascript

I'll let @johanvos speak to that. If this is desired, then it's probably best done as a follow-up, since I can't test the Android (or iOS) implementation.

[kevinrushforth]

I believe the scenario where jlink is used with an "old" JDK (e.g. 21) in combination with the most recent JavaFX release is very common. In that case, it seems to be required to prepend the module path with $JAVA_HOME/jmods, correct?

Yes.

If that is the case, we need to announce and explain it very well, as developers will run into it and get confused.

Agreed. I've marked this RFE as needing a release note, and will be sure to clarify this.

[johanvos]

Old version of classes netscape.javascript.JSObject and netscape.javascript.JSException seems to be delivered with JavaFX for Android [1]. Should these classes be removed? [1] : https://github.com/openjdk/jfx/tree/jfx23/modules/javafx.web/src/android/java/netscape/javascript

I'll let @johanvos speak to that. If this is desired, then it's probably best done as a follow-up, since I can't test the Android (or iOS) implementation.

Yes, those can be removed. There is more (ios/android) code that can be removed, or that even should be removed. I'll file a follow-up for this.

[johanvos]

Old version of classes netscape.javascript.JSObject and netscape.javascript.JSException seems to be delivered with JavaFX for Android [1]. Should these classes be removed? [1] : https://github.com/openjdk/jfx/tree/jfx23/modules/javafx.web/src/android/java/netscape/javascript

I'll let @johanvos speak to that. If this is desired, then it's probably best done as a follow-up, since I can't test the Android (or iOS) implementation.

Yes, those can be removed. There is more (ios/android) code that can be removed, or that even should be removed. I'll file a follow-up for this.

Created https://bugs.openjdk.org/browse/JDK-8339705 for this

[kevinrushforth]

After merging in the latest master yesterday, I discovered that the recently-enabled javac lint warnings and -Werror when compiling the test classes will fail the build with "removal" warnings for those tests that use JSObject, when I run using my JDK 24 build with the patch from openjdk/jdk#20555. This is a good thing, since it means that those tests were not using the version of jdk.jsojbect from JavaFX. This highlights the needs to use --upgradle-module-path in our unit tests when running with JDK 24, so I pushed a commit that does that.

I'm done with my testing, and will switch to creating the CSRs.

Reviewers: @arapte @jaybhaskar

[kevinrushforth]

I've created the CSRs for both the this PR and openjdk/jdk#20555.

[AnirvanSarkar]

I noticed that other JavaFX modules have make/build.properties file. It seems this file is used when co-bundling JavaFX with the JDK, when JDK is configured to be built with --with-import-modules option. Should we also add this file for jdk.jsobject module in JavaFX ?

[kevinrushforth]

Yes, I missed that. Thanks for catching it. I'll add a make/build.properties file, although the last time I checked, there were other problems in building a JavaFX bundle that could be used with --with-import-modules, so it may not be sufficient.

[kevinrushforth]

/integrate

[openjdk[bot]]

Going to push as commit f5b18adfa4151a7760b146a95ecea08b2b407d39. Since your change was applied there has been 1 commit pushed to the master branch:

• f71c3906d5da83adb69bf55d1e2854b8891dbefe: 8340003: Bump minimum JDK version for JavaFX to JDK 22

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@kevinrushforth Pushed as commit f5b18adfa4151a7760b146a95ecea08b2b407d39.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.