348993 - XPath expression errors when expr spans multiple instances

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Steve Speicher]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060816 Minefield/3.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060816 Minefield/3.0a1

When I have expressions that span multiple instances I get errors like:
  XForms Error (11): Error evaluating XPath expression: 'results2' 

Aaron says:
"I could recreate the problem with the latest trunk and I debugged it tonight.  The patch causing the problem went in on the 13th, a regression fix caused by the patch to bug 346362.  But the real problem is in our code.  The nsXFormsXPathAnalyzer is exposed in that it might potentially use the xpath evaluator for a different document than where the context node came from.  Basically could happen for most any complex expression that doesn't involve the first instance document in the xform."

Reproducible: Always

Comment 1

[Steve Speicher]

Attachment: testcase that has error message to console

Comment 2

[aaronr]

The insurance sample also fails.  You can tell because checking/unchecking the agreement checkbox does nothing (i.e. all of the 'case' toggle buttons already appear at the top of the form).

Comment 3

[aaronr]

Attachment: patch

I basically fixed this by ensuring that the xpath evaluator and the context node that could be used during an evaluation where from the same document.  One problem we had was where we take the evaluator and context node (document element) from the first instance document when we evaluate the @bind, @ref and @nodeset during EvaluateNodeBinding for controls yet they could actually reference a different instance.  This caused problems when we cached the evaluator and recursively looked through the expression for dependencies.  I fixed this by making sure the evaluator and the context node matched once we delved into the AnalyzeRecursively code.

We also did something similar during ProcessBind where we used the same evaluator for evaluating the bind's @nodeset expression AND for evaluating the MIP expressions.  We need to make sure that the evaluator for the MIP expressions come from the same document as the resulting nodeset nodes since they will be the context nodes for the MIP expressions.

Comment 4

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

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


>-  PRUint32 snapItem;
>-  for (snapItem = 0; snapItem < snapLen; ++snapItem) {
>+
>+  if (!snapLen) {
>+    return NS_OK;
>+  }
>+
>+  // We rightly assume that all the nodes in the nodeset came from the same
>+  // document.  So now we'll get the xpath evaluator from that document.  We
>+  // need to ensure that the context node for the evaluation of each MIP
>+  // expression and the evaluator for those expressions came from the same
>+  // document.  It is a rule for xpath.
>+  PRUint32 snapItem = 0;
>+  do {
>     rv = result->SnapshotItem(snapItem, getter_AddRefs(node));
>     NS_ENSURE_SUCCESS(rv, rv);
>-    
>+
>     if (!node) {
>       NS_WARNING("nsXFormsModelElement::ProcessBind(): Empty node in result set.");
>-      continue;
>+      ++snapItem;
>     }
>-    
>+  } while (!node && snapItem < snapLen);

Change this 'do-while' to 'for' or 'while'. You anyway check first that !snapLen.

Comment 5

[aaronr]

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

I'll fix Olli's comment when we get CVS access back.  In the meantime, can you r me doron?

Comment 6

[aaronr]

Attachment: patch for checkin

fixes Olli's comment

Comment 7

[aaronr]

checked into trunk

Comment 8

[aaronr]

leaving xf-to-branch on this now.  Need to look at 1.8.1 to see if it is needed there.

Comment 9

[aaronr]

removing xf-to-branch.  Doesn't look like patch that causes this problem made it into 1.8.  Testcase works fine.