Last Comment Bug 396179 - Cannot insert elements from xforms namespace using origin attribute on insert element
: Cannot insert elements from xforms namespace using origin attribute on insert...
Status: RESOLVED FIXED
: fixed1.8.1.12
Product: Core
Classification: Components
Component: XForms (show other bugs)
: unspecified
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Merle Sterling
:
Mentors:
Depends on:
Blocks: 410239
  Show dependency treegraph
 
Reported: 2007-09-14 08:14 PDT by Fraser
Modified: 2008-01-08 19:24 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test Case (2.28 KB, text/xml)
2007-09-14 08:29 PDT, Fraser
no flags Details
patch (2.01 KB, patch)
2007-09-26 12:17 PDT, Merle Sterling
bugs: review+
surkov.alexander: review+
Details | Diff | Review
patch2 (3.63 KB, patch)
2007-10-03 17:30 PDT, Merle Sterling
no flags Details | Diff | Review

Description Fraser 2007-09-14 08:14:49 PDT
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 2007-09-14 08:19:54 PDT
Merle, this is the bug from the newsgroup that you are already working on.
Comment 2 Fraser 2007-09-14 08:29:05 PDT
Created attachment 280898 [details]
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 2007-09-14 12:40:03 PDT
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 2007-09-26 12:17:10 PDT
Created attachment 282444 [details] [diff] [review]
patch

Removing check for xforms namespace in IsNodeAssocWithModel.
Comment 5 Olli Pettay [:smaug] 2007-09-26 14:27:47 PDT
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 2007-09-29 23:05:47 PDT
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 2007-10-02 12:23:14 PDT
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 2007-10-02 19:29:23 PDT
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 2007-10-03 17:30:27 PDT
Created attachment 283481 [details] [diff] [review]
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 2007-10-04 00:28:46 PDT
checked in
Comment 11 Fraser 2007-10-04 04:07:25 PDT
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 2007-11-12 13:58:06 PST
(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 2008-01-08 19:24:09 PST
checked into 1.8 branch via bug 410239.

Note You need to log in before you can comment on or make changes to this bug.