On Fri, 1 Nov 2024 13:26:25 GMT, Kevin Rushforth <[email protected]> wrote:
>> Andy Goryachev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review comments
>
> modules/javafx.web/src/main/java/com/sun/webkit/Utilities.java line 119:
>
>> 117: }
>> 118:
>> 119: return MethodHelper.invoke(method, instance, args);
>
> The removal of the try/catch is causing a test failure in
> `JavaScriptBridgeTest` because `InvocationTargetException` is now thrown
> rather than `InvocationTargetException::getCause`.
>
> Suggestion:
>
> try {
> return MethodHelper.invoke(method, instance, args);
> } catch (InvocationTargetException ex) {
> Throwable cause = ex.getCause();
> throw cause != null ? cause : ex;
> }
changed as suggested.
is this test being run as part of GHA or the headful test suite?
> modules/javafx.web/src/main/java/javafx/scene/web/HTMLEditorSkin.java line
> 835:
>
>> 833: @SuppressWarnings("removal")
>> 834: Image icon =
>> AccessController.doPrivileged((PrivilegedAction<Image>) () -> new
>> Image(HTMLEditorSkin.class.getResource(iconName).toString()));
>> 835: // button.setGraphic(new ImageView(icon));
>
> I see you removed this comment. Do you think there is no value in leaving it?
reverted here and below.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1620#discussion_r1825919662
PR Review Comment: https://git.openjdk.org/jfx/pull/1620#discussion_r1825918199
