Closed Bug 1135560 Opened 7 years ago Closed 7 years ago

Number.{parseInt, parseFloat} should be the same functions as global ones

Categories

(Core :: JavaScript: Standard Library, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: 446240525, Assigned: 446240525)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

js> Number.parseInt == parseInt
false // should be true
js> Number.parseFloat == parseFloat
false // should be true
Attached patch bug-1135560-v1.patch (obsolete) — Splinter Review
Assignee: nobody → 446240525
Attachment #8568400 - Flags: review?(till)
Comment on attachment 8568400 [details] [diff] [review]
bug-1135560-v1.patch

Review of attachment 8568400 [details] [diff] [review]:
-----------------------------------------------------------------

Great! f+ instead of r+ because I'd like to see a second version with the comment below addressed.

::: js/src/jsnum.cpp
@@ +1194,5 @@
>          return nullptr;
>      if (!JS_DefineFunctions(cx, global, number_functions))
>          return nullptr;
> +    /* Number.parseInt should be the same function object as global parseInt. */
> +    JSFunction *parseInt = JS_DefineFunction(cx, global, "parseInt", num_parseInt, 2, 0);

I think it'd be better to use the non-jsapi versions of JS_DefineFunction and JS_DefineProperty here. That way we don't have to atomize the names at all: they're already in the commonNames list. To do this, get the id like so:
RootedId id(cx, NameToId(cx->runtime()->commonNames->parseInt));

For DefineProperty, pass nullptr for the get and set op parameters.
Attachment #8568400 - Flags: review?(till) → feedback+
Attachment #8568400 - Attachment is obsolete: true
Attachment #8568534 - Flags: review?(till)
Comment on attachment 8568534 [details] [diff] [review]
bug-1135560-v2.patch

Review of attachment 8568534 [details] [diff] [review]:
-----------------------------------------------------------------

Excellent!
Attachment #8568534 - Flags: review?(till) → review+
https://hg.mozilla.org/mozilla-central/rev/617d8a7292f1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.