396179 - Cannot insert elements from xforms namespace using origin attribute on insert element

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Fraser]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
Build Identifier: 0.8.0.3

Discovered the bug when trying to develop an xform (main form) to edit another xform (instance form).  

I have a repeat element in the main form that lists the existing model elements and controls in the instance form.  I have created an instance of xform model and control elements to use as templates (controls instance).  Using the xforms insert element, with the origin attribute bound to a node in the controls instance, I should be able to insert a new xforms model or control element. 

Problem 1: With an xforms:instance element in the controls instance, the main form will not load.  The xforms extension appears to be trying to load the xform:instance element even though it is only supposed to be an xml template.  It also tries to load the xforms:instance elements in the instance form.

Problem 2: Taking out the xforms:instance elements in both the form and the controls instances, or associating them with a different namespace, the form will load. Clicking on the trigger to execute the insert action to insert a control (e.g. input) from the controls instance has no effect.  Tested in Orbeon xforms sandbox it does properly insert the control from the controls instance so it appears that the code is correct.

Reproducible: Always

Steps to Reproduce:
1. Create a simple xform
2. Create another xform that loads the first xform as an instance and has a repeat to list the controls in the instance xform
3. Create a another instance with a few xforms controls to be used as templates.
4. Add a trigger to the main form to insert an xform control from the controls instance into the xform instance (using origin bound to a node in the controls instance)
5. load the form and click on the trigger.  Nothing will happen. 
Actual Results:  
Clicking a trigger to insert and xforms control from a template instance into a form instance should insert the control.  

Expected Results:  
The insert action fails.

Comment 1

[aaronr]

Merle, this is the bug from the newsgroup that you are already working on.

Comment 2

[Fraser]

Attachment: Test Case

If the namespace for the elements in the controls instance is changed to my: then the insert action works.  It seems that the xforms extension is confused by the xforms: namespace in the instance.

Comment 3

[Merle Sterling]

This is not a problem with insert. It is a problem with the XPath instance function and the way we try to find the xf:model associated with the xf:instance. 

The xf:instance to be inserted is not associated with the xf:model to which it should be inserted and GetModel tries to find the default model and cannot. It fails with 'No default instance found'.

This is all because of the special case test in IsNodeAssocWithModel:
  // If the node is in the XForms namespace and XTF based, then it should
  //   be able to be handled by GetModel.  Otherwise it is probably an instance
  //   node in a instance document.
  //if (namespaceURI.EqualsLiteral(NS_NAMESPACE_XFORMS)) {
  //  nsCOMPtr<nsIDOMElement> element = do_QueryInterface(aNode);
  // nsCOMPtr<nsIModelElementPrivate> modelPriv = GetModel(element);
  //  modelNode = do_QueryInterface(modelPriv);
  //} else {
    // We are assuming that if the node coming in isn't a proper XForms element,
    //   then it is an instance element in an instance doc.  Now we just have
    //   to determine if the given model contains this instance document.
    nsCOMPtr<nsIDOMNode> instNode;
    nsresult rv =
      nsXFormsUtils::GetInstanceNodeForData(aNode, getter_AddRefs(instNode));
    if (NS_SUCCEEDED(rv) && instNode) {
      instNode->GetParentNode(getter_AddRefs(modelNode));
    }
  //}

If we do not check for the xforms namespace and always use GetInstanceNodeForData both cases (xforms and my namespace) work. I am not sure why that check is there or what ramifications removing it may have.

Comment 4

[Merle Sterling]

Attachment: patch

Removing check for xforms namespace in IsNodeAssocWithModel.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 282444 [details] [diff] [review]
patch

That code was added in Bug 278981, but apparently not reviewed by xforms people.

Comment 6

[alexander :surkov (:asurkov)]

Comment on attachment 282444 [details] [diff] [review]
patch

if you're both want to remove some functionality from the method then it's fine with me, just please correct description of the method to sync it with new code. with that r=me

Comment 7

[Merle Sterling]

What is wrong with the description of IsNodeAssocWithModel? It just describes the way an instance node can be associated with a model. We are just changing which function to use to determine the model and using GetInstanceNodeForData in all cases. 

The issue with this testcase was simply that it is not correct to use GetModel just because the node is in the xforms namespace. 

Comment 8

[alexander :surkov (:asurkov)]

Please correct me if I'm wrong. Now you check whether the given node is instance node and get a model for it, previously you additionaly checked whether the given node is xforms control (not a instance node) and returned a model the control is bound to. The comment for the method describes the second way too. You need to remove it.

Otherwise the node will be an XForms element
   * that was used as the context node of the XPath expression (i.e the
   * XForms control has an attribute that contains an XPath expression).
   * Form controls are associated with model elements either explicitly through
   * single-node binding or implicitly (if model cannot by calculated, it
   * will use the first model element encountered in the document).  The model
   * can also be inherited from a containing element like xforms:group or
   * xforms:repeat.

Comment 9

[Merle Sterling]

Attachment: patch2

Here's the patch with the comments for IsNodeAssocWithModel updated. Olli or Alex, can one of you check it in please?

Comment 10

[alexander :surkov (:asurkov)]

checked in

Comment 11

[Fraser]

Thanks so much guys!  Your efforts are much appreciated!! 

How can i now take advantage of this fix?  Do i download a nightly build?  When would this fix make it into the main xforms plug-in, next release?

Comment 12

[aaronr]

(In reply to comment #11)
> Thanks so much guys!  Your efforts are much appreciated!! 
> 
> How can i now take advantage of this fix?  Do i download a nightly build?  When
> would this fix make it into the main xforms plug-in, next release?
> 

this will make it into the next release.

Comment 13

[aaronr]

checked into 1.8 branch via bug 410239.