581194 - Multiple context() calls in the same expression cause an xpath evaluation error

Status: RESOLVED FIXED

(Core Graveyard :: XForms, defect)

Description

[Dion Sole]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2.7) Gecko/20100713 Firefox/3.6.7
Build Identifier: 

There's currently a bug in the nsXFormsUtils::TranslateExpression() function, such that an xpath expression that has more than one call to context() in it
e.g. "context()/@a + context()/@b" will fail.
This is due to only the first occurance of "context()" being replaced by TranslateExpression().

I'll attach a patch and testcase soon.

Reproducible: Always

Comment 1

[Dion Sole]

Attachment: Example with multiple context() calls in an expression

This file will cause an error when it tries to evaluate the "context()/@a + context()/@b" expression.

Comment 2

[Dion Sole]

Attachment: Patch to handle multiple context() calls

This just adds a loop to the TranslateExpression function so that it handles more than one context() call within an expression.

Comment 3

[aaronr]

Comment on attachment 459624 [details] [diff] [review]
Patch to handle multiple context() calls


>+
>+    if (start + contextExpr.Length() < remainder.Length()) {
>+      remainder = Substring(remainder, start + contextExpr.Length());
>+      start = remainder.Find(contextExpr);
>+
>+      if (start == kNotFound) {
>+        // No more context()s to translate, so append the remainder of the
>+        // string.
>+        aResult.Append(remainder);
>+      }
>     }
>-  } else {
>-    aResult = aExpression;
>-  }
>+    else {
>+      // "context()" was the last part of the string, so we've processed
>+      // the entire thing.

Code looks good.  But please put the else up a line with the closing if bracket so that it looks like '} else {' for code consistency.  Thanks.

Comment 4

[Dion Sole]

Attachment: Patch with else lined up

Same patch with the "else" change.

Comment 5

[Dion Sole]

Not sure if this needs any special checkin instructions or not...

Aaron's review+ carries over from the first version of the patch.

Comment 6

[alexander :surkov (:asurkov)]

landed - http://hg.mozilla.org/xforms/rev/2d318fd535cd