FXMLLoader: not supplying filename to script engine, not supplying event object as argument to script

Wed Nov 13 15:53:57 UTC 2019

Actually, in this case, sending an email to this list is the right way 
to start this discussion. I had meant to reply last week, but was out of 
town (at Devoxx), and busy with other things.

Before we take a look at a patch that implements what you propose to 
change, we need a little more information about what problems are being 
caused by the current behavior. Do you have a test program that isn't 
working correctly? Since this proposed fix changes visible behavior, we 
also need to consider whether there are any compatibility or 
documentation implications.

The next step would be to file a bug at bugreport.java.com [1]. The bug 
report should include a test case that demonstrates the problem. Also, 
if you haven't already done so, please read the CONTRIBUTING [2] guidelines.

Before a pull request, we will need a bug report with a standalone 
reproducible test case, submitted at https://bugreport.java.com/
  -- ideally a test case that could be turned into an automated test.

-- Kevin

On 11/13/2019 6:40 AM, Michael Paus wrote:
> Hi,
> have you considered directly contributing your proposed change via a 
> PR on
> <https://github.com/openjdk/jfx>? According to my experience this may 
> speed
> up things considerably but don't forget to follow the procedures as 
> outlined in
> <https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md>.
> --Michael
>
> Am 13.11.19 um 15:14 schrieb Rony G. Flatscher:
>> Hmm, not getting any feedback so far, so wondering if there are 
>> currently any Java developers who
>> take advantage of the ability of FXMLLoader to have FXML controllers 
>> implemented in any of the Java
>> javax.script languages?
>>
>> For those, who use scripting languages for FXML controllers the 
>> request that FXMLLoader adds both
>> entries, ScriptEngine.FILENAME (for debugging, logging) and 
>> ScriptEngine.ARGV () (for making the
>> event object available directly as an argument) into the engine 
>> Bindings, should be quite helpful
>> while developing and running the scripts.
>>
>> [Personally I am using the scripting engine ooRexx successfully for 
>> teaching oo programming from
>> scratch to JavaFX in a single semester (four hour lecture, eight 
>> ECTS) at a Business Administration
>> university. So these two missing features in the current FXMLLoader 
>> support for FXML controllers
>> would help tremendously, especially in case of coding errors as 
>> currently it is not clear from which
>> file the script that has an error comes from, making it extremely 
>> cumbersome and time consuming in
>> JavaFX applications that use multiple and complex FXML files.]
>>
>> Therefore I would kindly ask interested committers for mentoring the 
>> proposed changes. Enclosed
>> please find a simpler version of the patch that adds these missing 
>> features to the ENGINE_SCOPE
>> Bindings in the three locations where ScriptEngine.eval() gets 
>> invoked (it ).
>>
>> To comment this simple patch, maybe I should add a few remarks such 
>> that the context becomes clear:
>>
>>    * invoking a script via ScriptEngine.eval() will always be 
>> accompanied with a ScriptContext that
>>      usually maintains two Bindings (Maps):
>>
>>        o one, GLOBAL_SCOPE Bindings, for global entries (used e.g. 
>> for putting the FXML elements that
>>          have an fx:id attribute defined, such that scripts can get 
>> access to them, independent of a
>>          particular ScriptEngine) which can also be used for sharing 
>> values among different script
>>          invocations,
>>
>>        o one, ENGINE_SCOPE Bindings, usually used for individual 
>> invocations.
>>
>>    * while a FXML file gets processed sequentially by the FXMLLoader 
>> elements in the form of
>>      "<fx:script source="someScript.ext" />" will cause invoking the 
>> ScriptEngine.eval(Reader): this
>>      patch fetches the ENGINE_SCOPE Bindings and puts the value 
>> "someScript.ext" with the key
>>      ScriptEngine.FILENAME into it (cf. "@@ -1558,6 +1558,9 @@ public 
>> class FXMLLoader" and "@@
>>      -1582,6 +1585,8 @@ public class FXMLLoader" in the patch),
>>
>>    * if an event handler gets defined (e.g. with the event attribute 
>> "<fx:button ...
>>      onAction="someScript">") the FXMLLoader creates a 
>> ScriptEventHandler and stores "someScript" and
>>      the ScriptEngine for executing that script whenever the event 
>> fires.
>>
>>        o When an event fires, the current implementation creates a 
>> copy of the current ENGINE_SCOPE
>>          Bindings from the ScriptEngine's ScriptContext, adds its 
>> entries to it after saving the
>>          entry "event" with the ActionEvent object in it. It then 
>> changes the ScriptEngine's current
>>          ScriptContext such that it now uses the new copy of the 
>> Bindings as its ENGINE_SCOPE
>>          Bindings, runs the script using eval() and then restores the 
>> ScriptContext ENGINE_SCOPE
>>          Bindings.
>>
>>        o The supplied patch (cf. "@@ -1675,30 +1680,28 @@ public 
>> class FXMLLoader") instead will
>>          create a copy of the ENGINE_SCOPE Bindings only once at 
>> creation time (and puts the
>>          appropriate ScriptEngine.FILENAME into it using the name of 
>> the FXML file that defines the
>>          event script attribute) and will reuse that Bindings each 
>> time the handler gets invoked,
>>          after putting the actual "event" object and the respective 
>> ScriptEngine.ARGV entry into it.
>>          Using ScriptEngine.eval(String,Bindings) will use the 
>> supplied Bindings as the ENGINE_SCOPE
>>          Bindings for this invocation only, such that no restore is 
>> necessary upon return.
>>
>> As only entries get added to the engine Bindings that have not been 
>> used by FXMLLoader this simple
>> patch should not affect existing scripts. The patch has been tested 
>> and works.
>>
>> Maybe it helps the cause for applying this patch, if I point out that 
>> I have been active in a number
>> of opensource projects, including Apache's BSF which led to my 
>> participation as an expert in JSR-223
>> which originally defined the javax.script framework introduced with 
>> Java 6 (also authored a complete
>> ScriptEngine implementation with both, the javax.script.Compilable 
>> and the javax.script.Invocable
>> interfaces).
>>
>> So looking for interested committers who would be willing to mentor 
>> this patch. Please advise.
>>
>> ---rony
>>
>>
>>
>> On 06.11.2019 16:05, Rony G. Flatscher wrote:
>>> Using a script engine (javax.script.ScriptEngine) for implementing a 
>>> FXML controller there are two
>>> important information missing in the ScriptContext.ENGINE_SCOPE 
>>> Bindings supplied to the script used
>>> to eval() the script code:
>>>
>>>    * ScriptEngine.FILENAME
>>>        o This value denotes the file name from where the script code 
>>> was fetched that is being eval()'d.
>>>        o When debugging script controllers in a complex JavaFX 
>>> application it is mandatory to know
>>>          the file name the script code was taken from (as such 
>>> scripts could be called/run from
>>>          different FXML files). Also, in the case of script runtime 
>>> errors, usually the file name is
>>>          given by the script engine where the error has occurred to 
>>> ease debugging, such that it is
>>>          important to really supply the filename.
>>>            + Note: the 'location'-URL in ScriptContext.GLOBAL_SCOPE 
>>> refers the FXML file,  not to the
>>>              file that hosts the script that gets run if using the 
>>> "<fx:script" element where the
>>>              "source" attribute denotes the name of the script file.
>>>        o General solution: supply the appropriate 
>>> ScriptEngine.FILENAME entry to the
>>>          ScriptContext.ENGINE_SCOPE Bindings.
>>>
>>>    * ScriptEngine.ARGV
>>>        o This value denotes the arguments that get passed to the 
>>> script from Java in form of a Java
>>>          Array of type Object.
>>>        o When defining event handlers in FXML files in script code 
>>> the script does not get the
>>>          appropriate argument. Rather the script programmer needs to 
>>> access the
>>>          ScriptContext.ENGINE_SCOPE and fetch the entry named 
>>> "event" from there. Some script engines
>>>          may make the entries in the Bindings implicitly available 
>>> to the scripts, however this
>>>          cannot be expected by default. However, a ScriptEngine.ARGV 
>>> entry must be supplied to the
>>>          script by the script engine implementor, such that a script 
>>> coder gets the event object
>>>          argument in the script language's manner.
>>>        o General solution: supply the appropriate ScriptEngine.ARGV 
>>> Object array to the
>>>          ScriptContext.ENGINE_SCOPE Bindings.
>>>
>>> With these two changes not only writing controller scripts would be 
>>> eased, it also would
>>> instrumentate ScriptContext.ENGINE_SCOPE Bindings the way it was 
>>> intended by JSR-223.
>>>
>>> Enclosed please find a tested diff for FXMLLoader.java from the 
>>> current OpenJavaFX Master (version
>>> 14) that implements both, ScriptEngine.FILENAME entries for all 
>>> script invocations and in the case
>>> of a script event handler the appropriate ScriptEngine.ARGV entry 
>>> gets supplied, allowing the script
>>> to fetch the event object directly as an argument.
>>>
>>> As I have signed the OCA the code (in form of a git diff) can be 
>>> directly applied to FXMLLoader.java.
>>>
>>> If you need the patch in a different form, then please advise.
>>>
>>> ---rony
>>>
>>>
>