On Wed, 21 Jun 2023 13:34:28 GMT, Guillaume Tâche <[email protected]> wrote:
>> This fixes ResourceBundle loading by calling
>> `ResourceBundle.getBundle(value, Locale.getDefault())` when the loader
>> resources are null or their classloader is null.
>> Apparently the original author of the bug report said they would submit a
>> pull request, but it seems like it never happened.
>
> Guillaume Tâche has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Empty commit
Overall the fix and tests look fine.
I have a couple of comments on the fix.
modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java line 1101:
> 1099: loadListener.readInternalAttribute(localName,
> value);
> 1100: }
> 1101: final ResourceBundle loaderResources =
> FXMLLoader.this.resources;
Why to have this local temporary `loaderResources`?
We can simply use `this.resources` for readability.
modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java line 1106:
> 1104: } else {
> 1105: final ClassLoader cl =
> loaderResources.getClass().getClassLoader();
> 1106: resources = cl == null ?
Add a bracket around `cl == null`
-------------
Changes requested by aghaisas (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1158#pullrequestreview-1556626578
PR Review Comment: https://git.openjdk.org/jfx/pull/1158#discussion_r1280399572
PR Review Comment: https://git.openjdk.org/jfx/pull/1158#discussion_r1280400336
