On Thu, 1 Jul 2021 08:32:31 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
> Update JavaFX WebKit to GTK WebKit 2.32 (612.1). > There are two separate commits: > 1. > [commit](https://github.com/openjdk/jfx/commit/ed0baf5f23aed0d8aaa72645c8e03fde56d0f0cc) > : includes only native changes > 2. > [commit](https://github.com/openjdk/jfx/commit/b024c16551be7d929fa1d927d3be8f0fc75a64c3) > : includes non native changes > > This is change is targeted for JavaFX 18, shall be integrated after fork. The WebKit 612.1 update looks good. I did a full build and test on all three platforms, including lots of manual testing with HelloWebView. No problems found. I left a couple minor test comments, but they can be handled as follow-up P4 test bugs. NOTE: this cannot be integrated until after the `jfx17` fork next Thursday (scheduled for 16:00 UTC). modules/javafx.web/src/test/java/test/javafx/scene/web/HistoryTest.java line 100: > 98: // > 99: // check the title update > 100: // Since this uses a listener to read the title property, why did this check need to be removed? We might want to file a P4 test bug to look into adding it back in. To that end, do you think it is better to comment this block of code out rather than delete it? modules/javafx.web/src/test/java/test/javafx/scene/web/LoadNotificationsTest.java line 153: > 151: assertNull("WebEngine.title should be null", > web.getTitle()); > 152: } else { > 153: assertNotNull("WebEngine.title should be set", > web.getTitle()); Similar to the comment I made in `HistoryTest`, is it worth filing a P4 test bug to look at converting this to use a listener? If so, then leaving this in, but commented out might be a good idea. ------------- Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/560
