setvalue actions can only use absolute references

[ggalmazor]

Software versions

JavaRosa v2.11.2

Problem description

Consider the following form:

<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml">
    <h:head>
        <h:title>relative-current-ref</h:title>
        <model>
            <instance>
                <data id="relative-current-ref-repeat">
                    <meta>
                        <instanceID/>
                    </meta>
                    <repeat_group>
                        <person/>
                        <selected_person/>
                    </repeat_group>
                </data>
            </instance>
            <bind nodeset="/data/repeat_group/person" type="string"/>
            <bind nodeset="/data/repeat_group/selected_person" type="string"/>
            <bind calculate="concat('uuid:', uuid())" nodeset="/data/meta/instanceID" readonly="true()" type="string"/>
        </model>
    </h:head>
    <h:body>
        <repeat nodeset="/data/repeat_group">
            <group>
                <label>Top level group</label>
                <input ref="/data/repeat_group/person">
                    <label>Person</label>
                </input>
                <input ref="/data/repeat_group/selected_person">
                    <label>Selected Person</label>
                    <setvalue event="xforms-value-changed" ref="curren()/../person">Value changed!</setvalue>
                </input>
            </group>
        </repeat>
    </h:body>
</h:html>

(don't mind the field names)

The key thing to consider here is that /data/repeat_group/selected_person uses a setvalue action to set the value of a field taking it from a sibling, whenever that sibling changes its value.

The parser is complaining about this form with this error: XPath evaluation: Attempt to parent past the root node current()../person.

Steps to reproduce the problem

I've pushed a branch to my fork with a form and a test class that reproduces the error: https://github.com/ggalmazor/javarosa/blob/set-value-issue/test/org/javarosa/xpath/expr/XPathPathExprCurrentSetValueTest.java

Expected behavior

The field using a setvalue action should receive the value from its sibling without failures.

Other information

In #349 (comment) @lognaturel comments:

We already actually have a good form for this -- nested-setvalue-action-with-repeats.xml. But it abuses the issue at opendatakit.github.io/xforms-spec/#a-big-deviation-with-xforms and uses an absolute reference (a big reason we want to make the change to using relative paths in repeats in XLSForm/pyxform#187).

I changed /data/repeat/cost_timestamp to ../cost_timestamp, debugged and see that the problem is in XFormParser.parseSetValueAction -- dataRef = FormDef.getAbsRef(dataRef, TreeReference.rootRef()) shouldn't always use the root ref for anchoring, it should use the current context. I haven't looked at a fix yet.

[ggalmazor]

Hi, @lognaturel.

When you say that "it should use the current context", what do you mean by it?

I see that FormDef.getAbsRef() takes an XPathReference as a first arg and a parent TreeReference as a second arg. I understand that we need to change the second arg and intuitively I'd like to pass the TreeReference of the parent input element, but I don't find any way to produce a TreeReference from an Element.

I could manually produce an absolute TreeReference by navigating the element and its ancestors, but I was hoping that this would be already implemented somewhere, although I can't find it. Any suggestions?

[lognaturel]

When you say that "it should use the current context", what do you mean by it?

The second param to dataRef = FormDef.getAbsRef(dataRef, TreeReference.rootRef()) means that everything is anchored to the root. The rules for defining the current context node are at https://www.w3.org/TR/2001/WD-xforms-20011207/slice7.html#expr-eval. So yes, you're right that it should inherit from the parent input element. But now I've gotten myself confused -- should the ref on the input also be relative because it's in a repeat?

You've looked at XFormPaser.getAbsRef(IDataReference ref, IFormElement parent)? It calls getFormElementRef and gets the reference from the bind which seems like the way to go. Then it doesn't matter whether or not the input's ref is relative (which I do think it should be. I've verified that the test still passes if it's changed to cost which seems right -- we what the input to be for the cost for which ever repeat we're in).

[ggalmazor]

See https://github.com/ggalmazor/javarosa/tree/issue_360_fix_setvalue_with_current

[ggalmazor]

Adding #349 (comment) here since I think it's relevant:

I think it'd be good to have an example like that with a relative reference. Not sure if it should really use current().

Maybe something to consider is to have a series of tests that demonstrate where expressions with current() evaluate to the same thing as expressions with . and where they don't. That might be a better place for comments we've been discussing at /pull/361/files#r211469938 (though you're right that adding one by the current case in XPathPathExpr would be helpful regardless).