On Wed, 13 Mar 2024 16:38:14 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Alexander Zuev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Minor changes based on review comments.
>
> test/jdk/javax/swing/JEditorPane/bug4694598.java line 64:
>
>> 62: } catch (IOException ioe){
>> 63: throw new RuntimeException("Could not create html file to
>> embed", ioe);
>> 64: }
>
> Move creating the file to `main` method before setting up GUI and let
> `IOException` escape from main.
This is try-with-resources so if i will do it in main i will have to add
synchronizing and closing of writer which is a strange trade-off so i would
have to do try block anyways.
> test/jdk/javax/swing/JEditorPane/bug4694598.java line 74:
>
>> 72: String html = "<HTML> <BODY>" +
>> 73: "<FRAMESET cols=\"100%\">" +
>> 74: "<FRAME src=\"" + frameContentUrl + "\">" +
>
> Suggestion:
>
> "<FRAME src="" + frameContentFile.toUri()+ "">" +
>
> Isn't it enough? Alternatively, `"file:/" +
> frameContentFile.toAbsolutePath()` produces the same result as
> `frameContentFile.toUri().toURL()`.
>
> Another option is to convert the `Path` to `URL` in the `main` method before
> calling `setupGUI` and remove try-catch blocks.
Again, i do not mind the try/catch block and using URI instead of URL gives
different result and i am not sure the original bug would be reproducible with
URI which will make it a strange case of the regression test.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18259#discussion_r1523895158
PR Review Comment: https://git.openjdk.org/jdk/pull/18259#discussion_r1523900378
