On Sat, 11 Jul 2026 18:01:41 GMT, Petr Štechmüller <[email protected]> wrote:
>> ProxyBuilder is used by the FXML loader to instantiate classes whose >> constructors are annotated with `@NamedArg`. When such a class also exposes >> a read-only `Map` property (e.g. getProperties() — the same pattern used by >> `javafx.scene.Node`), setting child elements under that property in FXML >> caused incorrect behaviour or a runtime error. >> >> **Root cause:** _getReadOnlyProperty()_ always returned an >> `ArrayListWrapper` regardless of the actual **getter** return type. When the >> getter returns a `Map`, an `ArrayListWrapper` is the wrong container and the >> entries are never transferred to the real map on the object. >> >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Petr Štechmüller has updated the pull request incrementally with one > additional commit since the last revision: > > Remove SuppressWarnings annotation from tests Looks good. The test is still `@Disabled`, that need to be fixed and I suggest to remove the commented out code, because comma separated values were never supported by FXML so I can't see how this is a bug. More details inline. modules/javafx.fxml/src/test/java/test/com/sun/javafx/fxml/builder/ProxyBuilderTest.java line 111: > 109: > 110: @Test > 111: @Disabled must be removed so that the test will run modules/javafx.fxml/src/test/java/test/com/sun/javafx/fxml/builder/ProxyBuilderTest.java line 117: > 115: pb.put("b", 456); > 116: ImmutableClass result = (ImmutableClass) pb.build(); > 117: assertEquals(123, result.a, 0.00001); I just saw that below, you were using `1e-5f`, lets use it here and below as well Suggestion: assertEquals(123, result.a, 1e-5f); modules/javafx.fxml/src/test/java/test/javafx/fxml/ReadOnlyCollectionsCreationTest.java line 80: > 78: assertEquals("hello", result.child, > 79: "FXMLLoader must unwrap the single ArrayList element and > pass it as the scalar @NamedArg constructor argument"); > 80: remove the empty newline modules/javafx.fxml/src/test/resources/test/javafx/fxml/readonly_collections_creation_a.fxml line 30: > 28: <?import test.javafx.fxml.ClassWithCollections?> > 29: <ClassWithCollections xmlns:fx="http://javafx.com/fxml";> > 30: <!-- names="a, b, c"--> same here, remove commented out code modules/javafx.fxml/src/test/resources/test/javafx/fxml/readonly_collections_creation_b.fxml line 31: > 29: <ClassWithCollections2 xmlns:fx="http://javafx.com/fxml"; > 30: id="RT_8203870"> > 31: <!-- names="a, b, c"--> I think we should remove the commented out code. I don't think that comma separated values are supported. In XML (and FXML), there is no such things as comma separated values equals List/Array elements. So I would say lets remove and rather discuss this in the mailing list after this PR is merged. ------------- PR Review: https://git.openjdk.org/jfx/pull/2167#pullrequestreview-4680248222 PR Review Comment: https://git.openjdk.org/jfx/pull/2167#discussion_r3566530026 PR Review Comment: https://git.openjdk.org/jfx/pull/2167#discussion_r3566540918 PR Review Comment: https://git.openjdk.org/jfx/pull/2167#discussion_r3566533037 PR Review Comment: https://git.openjdk.org/jfx/pull/2167#discussion_r3566538146 PR Review Comment: https://git.openjdk.org/jfx/pull/2167#discussion_r3566537817
