The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
9 years ago
9 months ago

People

(Reporter: Merle Sterling, Assigned: Merle Sterling)

Tracking

({fixed1.8.1.13})

Trunk
x86
Windows XP
fixed1.8.1.13
Bug Flags:
wanted-fennec1.0 ?
in-testsuite ?
in-litmus ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Assignee)

Description

9 years ago
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

Updated

9 years ago
Assignee: nobody → msterlin
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 1

9 years ago
Created attachment 301706 [details]
testcase: power()
(Assignee)

Updated

9 years ago
Attachment #301706 - Attachment mime type: text/xml → application/xhtml+xml
(Assignee)

Comment 2

9 years ago
Created attachment 301707 [details]
testcase: random()
(Assignee)

Comment 3

9 years ago
Created attachment 301708 [details]
testcase: compare()
(Assignee)

Comment 4

9 years ago
Created attachment 301711 [details] [diff] [review]
patch
Attachment #301711 - Flags: review?(aaronr)
(Assignee)

Updated

9 years ago
Status: NEW → ASSIGNED

Comment 5

9 years ago
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+
(Assignee)

Comment 6

9 years ago
(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.


(Assignee)

Comment 7

9 years ago
Created attachment 302229 [details] [diff] [review]
patch2

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+

Comment 8

9 years ago
checked into trunk for msterlin.  Now we need the 1.8 branch version of the patch.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch-11
(Assignee)

Comment 9

9 years ago
Created attachment 302917 [details] [diff] [review]
patch for 1.8 branch
Attachment #302917 - Flags: review?(aaronr)
(Assignee)

Updated

9 years ago
Attachment #302917 - Flags: review?(Olli.Pettay)

Updated

9 years ago
Attachment #302917 - Flags: review?(aaronr) → review+
Attachment #302917 - Flags: review?(Olli.Pettay) → review+

Comment 10

9 years ago
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+
(Assignee)

Comment 12

9 years ago
Created attachment 303416 [details] [diff] [review]
patch2 for 1.8 branch

Removing length check for compare().
Attachment #302917 - Attachment is obsolete: true

Comment 13

9 years ago
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+

Comment 15

9 years ago
Created attachment 307384 [details] [diff] [review]
patch checked into 1.8

patch checked into 1.8.  Same code as previous patch, just updated to the branch.
Attachment #303416 - Attachment is obsolete: true

Comment 16

9 years ago
checked into 1.8 branch for msterlin
Keywords: fixed1.8.1.13
Whiteboard: xf-to-branch-11

Updated

7 years ago
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.