[Webkit-unassigned] [Bug 32698] [JSC] Date binding support

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=32698





--- Comment #12 from TAMURA, Kent <tkent at chromium.org>  2009-12-20 22:06:20 PST ---
I'll commit manually with the following changes.

(In reply to comment #11)
> (From update of attachment 45235 [details])
> > +JSValue jsDateOrNull(ExecState* exec, double value)
> > +{
> > +    if (isfinite(value))
> > +        return new (exec) DateInstance(exec, value);
> > +    return jsNull();
> > +}
> 
> We normally prefer to do early exit for failures. So if (!isfinite) return
> jsNull. We assume that the success case code might later get longer so we want
> it to be non-indented.

Fixed.

> > +    if (value.isNumber())
> > +        return value.toNumber(exec);
> 
> I think we still need to think through how we want to handle various types.
> It's good that actual numbers will now work, but there are other things like
> number wrappers (made with expressions like "new Number(1)") and strings.
> Generally we want to consider all JavaScript types when writing a conversion
> function like this.
> 
> If you have already gotten true from isNumber, you should not call toNumber.
> Instead you call uncheckedGetNumber. Or you can combine the two operations into
> one, using the getNumber function which returns a boolean to indicate whether
> the value is a number or not.

I changed it to uncheckedGetNumber().

> > +    // NaN if the value is null, undefined, or non-Date object.
> > +    double valueToDate(JSC::ExecState*, JSC::JSValue);
> 
> The comment is too specific. I would say "NaN if the value can't be converted
> to a date", without giving the specifics of how the date conversion works.

Fixed.

> I really think we need a specification of how the conversion to date works for
> all types. And this unambiguous specification should be in whatever document
> defines the JavaScript binding. Maybe WebIDL or HTML5. That would include both
> how the conversion works, and also what behavior is expected when the value
> can't be converted (a specific exception raise, perhaps) I am not happy with
> all the current choices reflected in our valueToDate function.

I made a bug for this issue: Bug#32809.

And bugs for other bindings:
V8: Bug#32699
ObjC: Bug#32810
COM: Bug#32811

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.