XPath expression errors when expr spans multiple instances

RESOLVED FIXED

Status

RESOLVED FIXED
13 years ago
3 years ago

People

(Reporter: sspeiche, Assigned: aaronr)

Tracking

Trunk
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
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
(Reporter)

Comment 1

13 years ago
Created attachment 234211 [details]
testcase that has error message to console
(Assignee)

Updated

13 years ago
Assignee: xforms → aaronr
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 2

13 years ago
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).
(Assignee)

Comment 3

13 years ago
Created attachment 234322 [details] [diff] [review]
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.
Attachment #234322 - Flags: review?(Olli.Pettay)
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.
Attachment #234322 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Comment 5

13 years ago
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?
Attachment #234322 - Flags: review?(doronr)
Attachment #234322 - Flags: review?(doronr) → review+
(Assignee)

Comment 6

13 years ago
Created attachment 234472 [details] [diff] [review]
patch for checkin

fixes Olli's comment
Attachment #234322 - Attachment is obsolete: true
(Assignee)

Comment 7

13 years ago
checked into trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch
(Assignee)

Comment 8

13 years ago
leaving xf-to-branch on this now.  Need to look at 1.8.1 to see if it is needed there.
(Assignee)

Comment 9

12 years ago
removing xf-to-branch.  Doesn't look like patch that causes this problem made it into 1.8.  Testcase works fine.
Whiteboard: xf-to-branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.