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

[bugzilla-daemon at webkit.org]

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


TAMURA, Kent <tkent at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #45166|0                           |1
        is obsolete|                            |
  Attachment #45235|                            |review?, commit-queue-
               Flag|                            |




--- Comment #9 from TAMURA, Kent <tkent at chromium.org>  2009-12-19 06:24:12 PST ---
Created an attachment (id=45235)
 --> (https://bugs.webkit.org/attachment.cgi?id=45235)
Proposed patch (rev.3)

(In reply to comment #8)
> (From update of attachment 45166 [details])
> > +    return (isnan(value) || isinf(value)) ? jsNull() : new (exec) DateInstance(exec, value);
> 
> I just remembered that there's a function called isfinite that combines both
> these checks into one. I think you should use that instead.

Done.

> Also, does this expression compile? I'm surprised, happily, that it works given
> the two sides of the ?: expression have different types.

Yes. I confirmed with Xcode and VS2005. But I changed it to a "if" statement
just in case.

>     if (!value.inherits(&DateInstance::info))

Done.

> > +    // NaN if the value if null or undefined.
> > +    double valueToDateWithUndefinedOrNullCheck(JSC::ExecState*, JSC::JSValue);
> 
> I think it's strange to call out the undefined/null check in the name of the
> function, since you also check for any other kind of non-date object and handle
> them the same way you would undefined or null.

Renamed to valueToDate().


> It would be more typical in JavaScript API to try to convert the type, for
> example make a date from the number if the argument is a number, and perhaps
> the same thing for a string. Are you sure you have the right expected behavior
> here for numeric values? Does the behavior you chose come from the HTML5 or
> WebIDL specification?

Neither HTML5 nor WebIDL mention it. I think accepting numbers is reasonable,
and changed the code for it.

> > +    // FIXME: This is a temporal implementation to check Date binding.
> 
> Slight grammar mistake. This should be "temporary" instead of "temporal".

Fixed.

> > +#if defined(LANGUAGE_JAVASCRIPT) && LANGUAGE_JAVASCRIPT
> > +                 attribute Date            valueAsDate setter raises(DOMException);
> > +#endif
> 
> It's a little ugly to have to put LANGUAGE_JAVASCRIPT around any use of Date.
> It probably would be easy to make the Objective-C binding handle dates, for
> example.

Indeed.  I'll make patches for Objective-C and COM soon. I already have V8
support code.

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