JDK-8134600 : Can't pass ObservableList as argument using FXML
  • Type: Bug
  • Component: javafx
  • Sub-Component: fxml
  • Affected Version: 8u40,9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2015-08-27
  • Updated: 2020-01-31
  • Resolved: 2016-10-16
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 JDK 9
8u152Fixed 9Fixed
Related Reports
Relates :  
JDK-8097347 - [FXML] ProxyBuilder does not support read-only collections
Relates :  
JDK-8094318 - FXMLLoader fails to load attribute styleClass on Spinner
Description
Reproducible: always
Platform-specific: no
Is a regression: yes (works fine in 8u20, 8u31 GA doesn't work in 8u40)
Steps to reproduce: run attached sample
Expected result: two similar bar charts
Actual result: only one bar chart, strange exception thrown
Comments
Approved to backport to 8u-dev for 8u122. If the patch applies cleanly (which it should), then no need to post a new webrev.
18-10-2016
As this is a regression, we should backport this to 8u.
18-10-2016
Changeset: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/58ac4dae137c
16-10-2016
I bisected this and discovered that the following fix done in 8u60 eliminated the need for the original fix for JDK-8097347 : changeset: 8748:b06d15751d1f user: jgiles date: Tue Mar 24 14:49:54 2015 +1300 summary: RT-40335: FXMLLoader fails to load attribute styleClass on Spinner where RT-40335 == JDK-8094318 That fix was also in ProxyBuilder.java as follows: --- a/modules/fxml/src/main/java/com/sun/javafx/fxml/builder/ProxyBuilder.java +++ b/modules/fxml/src/main/java/com/sun/javafx/fxml/builder/ProxyBuilder.java @@ -548,6 +548,8 @@ if (argStr instanceof Collection) { Collection from = (Collection) argStr; to.addAll(from); + } else { + to.add(argStr); } } } I further confirmed by doing an hg update to the changeset just prior to the one that implemented the original fix for JDK-8097347 (at which point BarChartFXML works, but the test case from JDK-8097347 does not) and apply just the above patch from JDK-8094318 then everything works as expected. So the bottom line is that the following lines from the original fix for JDK-8097347 can be reverted as follows to fix this bug: --- a/modules/javafx.fxml/src/main/java/com/sun/javafx/fxml/builder/ProxyBuilder.java +++ b/modules/javafx.fxml/src/main/java/com/sun/javafx/fxml/builder/ProxyBuilder.java @@ -447,10 +447,6 @@ } } - if (Collection.class.isAssignableFrom(type)) { - return val; - } - if (ArrayListWrapper.class.equals(val.getClass())) { // user given value is an ArrayList but the constructor doesn't // accept an ArrayList so the ArrayList comes from
14-10-2016
I can confirm that Martin's change is no longer necessary. Commenting out the three lines of code in ProxyBuilder.java fixes the regression with not being able to use ObservableList, and yet the test case from JDK-8097347 still passes. I tried it in both 9-dev and 8u-dev and got the same results. I also note that Andrey Rusakov's first proposed fix was effectively the same fix (rather than removing the three lines, he just moved them after the following block). Vadim raised the concern that this would regress JDK-8097347, but that does not seem to be the case. Running "java fxmlproxybuilder.Test" from the attachment to JDK-8097347 works without the three lines in question. I didn't dig into it enough to understand it. My guess is that something else changed post 8u40 that obviated the original fix for JDK-8097347. It would be good to know what that was. A couple ways I can think of to find out are: 1) look at JBS to see what fxml bugs were fix in 8uXX after JDK-8097347; or 2) Use 'hg bisect' to find the changeset that eliminated the need for the original fix.
13-10-2016
I should start by saying I own FXML by default, rather than because I've had anything but the barest of interactions with it. Therefore, what I'm about to say may be entirely incorrect...You've been warned. The basic problem appears to be that when we bring in an ObservableArrayList, FXML wraps this in an ArrayListWrapper. The code comments say that ArrayListWrapper is a "wrapper for ArrayList which we use to store read-only collection properties in". I'm not entirely clear what makes the observable list we pass in via FXML qualify as read-only (I get the feeling it shouldn't be truly read-only in the sense of FXCollections.unmodifiableObservableList, just as a notion inside FXML). What essentially happens in the test code on this issue is that the entire ObservableArrayList, as specified in the FXML, is inserted inside the ArrayListWrapper. The end result is that the ArrayListWrapper contains a single element - the ObservableArrayList. Everything was going well until Martin made the change in JDK-8097347. This simple change appears to have had the effect of allowing the ProxyBuilder code inside the FXML module to directly return the ArrayListWrapper, which is not what is expected elsewhere in FXML. In other words, he simply added the following code: if (Collection.class.isAssignableFrom(type)) { return val; } And this is sufficient for the ArrayListWrapper to be returned, as ArrayListWrapper extends ArrayList. Unfortunately, the code we really want to hit is immediately after that if statement: if (ArrayListWrapper.class.equals(val.getClass())) { // user given value is an ArrayList but the constructor doesn't // accept an ArrayList so the ArrayList comes from // the getTemporaryContainer method // we take the first argument List l = (List) val; return l.get(0); } Slightly confusingly, the code Martin added, when commented out, still allows the test attached to JDK-8097347 to pass. All unit tests continue to pass as well. In other words, I'm not sure if the change made in JDK-8097347 is still necessary. Having said that, there are ways to keep that code in (if desired) and still properly unwrap the ArrayListWrapper. We can do this in the BeanAdapter with a special case for when the type is ObservableList.class and the value is ArrayListWrapper. I have attached a proof of concept webrev at the link below [1]. This isn't intended to be the final proposed patch (as I'm not convinced it is necessary given that we can seemingly remove the change Martin made), but I wanted to capture the alternate path anyhow. What I'd like to see now is a discussion on how we proceed. I presume we try to understand why that change from Martin is no longer necessary. [1] http://cr.openjdk.java.net/~jgiles/8134600/
23-08-2016
arusakov Andrey Rusakov added a comment - 5 minutes ago No, I don't have enough time for that. I think, that task should have open deadline or assigned to developer.
29-06-2016
Andrey, Unfortunately this will break applications which use read-only collection properties, please see the test case attached to the related bug JDK-8097347. The unit test in that fix doesn't actually test this behavior.
22-04-2016
Ok, fixed that: http://cr.openjdk.java.net/~arusakov/8134600/webrev.01
22-04-2016
I'll let Vadim comment on the fix itself. One thing you need to fix is that your patch contains trailing whitespace: $ gradle checkrepo ... :checkrepo modules/fxml/src/main/java/com/sun/javafx/fxml/builder/ProxyBuilder.java :trailingWhitespace: Found 1 whitespace or executable issues To correct, use bash tools/scripts/checkWhiteSpace -F :checkrepo FAILED
21-04-2016
Well, the reason was not in BarChart and not in BeanAdapter.coerce, but in ProxyBuilder.getUserValue where check for Collection class was performed before check for ArrayListWrapper class, so check for ArrayListWrapper class could not be fulfilled at all. Case is covered by FxmlTests/test/fxmltests/functional/staticPropertyLoadTest.java#coerceSlot test, so there is probably no need to create a new one. http://cr.openjdk.java.net/~arusakov/8134600/webrev.00/
21-04-2016
Since this is a regression introduced in an 8 update release, it might be a good candidate for a backport to 8u.
20-04-2016
The reason of failure is BeanAdapter.coerce, which can't coerce java.util.ArrayList to javafx.collections.ObservableList.
20-04-2016
RULE "FxmlTests/test/fxmltests/functional/staticPropertyLoadTest/coerceSlot" Exception java.lang.RuntimeException: Cannot create instance of javafx.scene.chart.BarChart with given set of properties: [yAxis, xAxis, data, id, title]
29-10-2015
Testsuite: FxmlTests Test: test/fxmltests/app/staticPropertyLoadApp.java#coerceSlot
27-10-2015
Andrey, is this found by existing test(s)? Can you please provide test name(s)?
27-10-2015
Windows 7 64 bit, fails with 64 bit 8u40 b01.
15-10-2015
escape-old label is added, since problem occurs since 8u40.
02-10-2015