Closed Bug 415963 Opened 14 years ago Closed 14 years ago

[1.1] Implement XPath number functions: power, random, and compare

Categories

(Core Graveyard :: XForms, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: msterlin, Assigned: msterlin)

Details

(Keywords: fixed1.8.1.13)

Attachments

(5 files, 3 obsolete files)

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.1.4322; .NET CLR 2.0.50727)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008020507 Minefield/3.0b4pre

We need to implement the XPath number functions power(), random(), and compare() for XForms 1.1.



Reproducible: Always
Assignee: nobody → msterlin
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached file testcase: power()
Attachment #301706 - Attachment mime type: text/xml → application/xhtml+xml
Attached file testcase: random()
Attached file testcase: compare()
Attached patch patch (obsolete) — Splinter Review
Attachment #301711 - Flags: review?(aaronr)
Status: NEW → ASSIGNED
Comment on attachment 301711 [details] [diff] [review]
patch

>Index: nsXFormsXPathFunctions.cpp
>===================================================================

>+NS_IMETHODIMP
>+nsXFormsXPathFunctions::Random(PRBool aSeed, double *aResult)
>+{
>+    if (aSeed) {
>+      // initialize random seed.
>+      srand (0);

You can use PR_GetRandomNoise to come up with a seed to use instead of using 0 all the time.

With that, r=me
Attachment #301711 - Flags: review?(aaronr) → review+
(In reply to comment #5)
> (From update of attachment 301711 [details] [diff] [review])
> >Index: nsXFormsXPathFunctions.cpp
> >===================================================================
> >+NS_IMETHODIMP
> >+nsXFormsXPathFunctions::Random(PRBool aSeed, double *aResult)
> >+{
> >+    if (aSeed) {
> >+      // initialize random seed.
> >+      srand (0);
> You can use PR_GetRandomNoise to come up with a seed to use instead of using 0
> all the time.
> With that, r=me

That should have been srand(time(NULL)) but I'll use PR_GetRandomNoise.


Attached patch patch2Splinter Review
Merging with the latest version of the file on the trunk; using PR_GetRandomNoise to generate a seed.
Attachment #301711 - Attachment is obsolete: true
Attachment #302229 - Flags: review?(Olli.Pettay)
Attachment #302229 - Flags: review?(Olli.Pettay) → review+
checked into trunk for msterlin.  Now we need the 1.8 branch version of the patch.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch-11
Attached patch patch for 1.8 branch (obsolete) — Splinter Review
Attachment #302917 - Flags: review?(aaronr)
Attachment #302917 - Flags: review?(Olli.Pettay)
Attachment #302917 - Flags: review?(aaronr) → review+
Attachment #302917 - Flags: review?(Olli.Pettay) → review+
Comment on attachment 302917 [details] [diff] [review]
patch for 1.8 branch

Jonas, can you please review so that we can get approval to put it into 1.8?  Thanks.
Attachment #302917 - Flags: superreview?(jonas)
Comment on attachment 302917 [details] [diff] [review]
patch for 1.8 branch

>+      double result = 0;
>+      if (string1.Length() > string2.Length()) {
>+        result = 1;
>+      } else if (string1.Length() < string2.Length()) {
>+        result = -1;
>+      } else {
>+        result = strcmp(NS_ConvertUTF16toUTF8(string1).get(),
>+                        NS_ConvertUTF16toUTF8(string2).get());
>+      }

This looks wrong. Is "z" < "aa" really right? Spec says lexicographic comparison.

sr=me with that fixed or checked.
Attachment #302917 - Flags: superreview?(jonas) → superreview+
Attached patch patch2 for 1.8 branch (obsolete) — Splinter Review
Removing length check for compare().
Attachment #302917 - Attachment is obsolete: true
Comment on attachment 303416 [details] [diff] [review]
patch2 for 1.8 branch

Asking for approval for 1.8 branch.  This code is reachable only by xforms xpath evaluators which should only ever be instantiated by the xforms extension.  This should have no impact on any other xpath user.
Attachment #303416 - Flags: approval1.8.1.13?
Comment on attachment 303416 [details] [diff] [review]
patch2 for 1.8 branch

Seems a little odd to be adding features to the branch -- xforms users will now have to require a specific minimum version.

approved for 1.8.1.13, a=dveditz for release-drivers
Attachment #303416 - Flags: approval1.8.1.13? → approval1.8.1.13+
patch checked into 1.8.  Same code as previous patch, just updated to the branch.
Attachment #303416 - Attachment is obsolete: true
checked into 1.8 branch for msterlin
Keywords: fixed1.8.1.13
Whiteboard: xf-to-branch-11
Flags: wanted-fennec1.0?
Flags: in-testsuite?
Flags: in-litmus?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.