I think we do JSObject* for now, which is completely sucky.

Comment on attachment 744197 [details] [diff] [review] Part 1 updated to not assume that our JS apis are sane I'm not sure about the coding style. { in class and method declaration should go to its own line and params should be in form aFoo, but bindings code use some odd style. >+Date::SetTimeStamp(JSContext* cx, JSObject* objArg) >+{ Couldn't you take JS::Handle<JSObject*> as param. >+Date::ToDateObject(JSContext* cx, JS::Value* vp) const And here maybe JS::MutableHandle<Value> vp r- until rooting fixed or explained why we want more old style API.

Comment on attachment 743801 [details] [diff] [review] part 2. Implement WebIDL parser support for Date. ># HG changeset patch ># User Boris Zbarsky <bzbarsky@mit.edu> ># Date 1367350309 14400 ># Node ID dacadac04a77b018611b8c3e2349f2e2a3863294 ># Parent 0e8a07f35791f91b38157836ad3e9aaed8be3f2e >Bug 742206 part 2. Implement WebIDL parser support for Date. r=smaug > >diff --git a/dom/bindings/parser/WebIDL.py b/dom/bindings/parser/WebIDL.py >--- a/dom/bindings/parser/WebIDL.py >+++ b/dom/bindings/parser/WebIDL.py >@@ -1268,23 +1268,23 @@ class IDLType(IDLObject): > > def isDictionary(self): > return False > > def isInterface(self): > return False > > def isAny(self): >- return self.tag() == IDLType.Tags.any and not self.isSequence() >+ return self.tag() == IDLType.Tags.any and not self.isSequence() and not self.isArray() > > def isDate(self): >- return self.tag() == IDLType.Tags.date >+ return self.tag() == IDLType.Tags.date and not self.isSequence() and not self.isArray() > > def isObject(self): >- return self.tag() == IDLType.Tags.object >+ return self.tag() == IDLType.Tags.object and not self.isSequence() and not self.isArray() This could use some comments. Why "not self.isSequence() and not self.isArray()" Based on https://bugzilla.mozilla.org/show_bug.cgi?id=768684#c1 it hasn't been too clear to you either ;)

Comment on attachment 744197 [details] [diff] [review] Part 1 updated to not assume that our JS apis are sane Ok, I guess this makes sense after all in the correct codegen. At least in the case I was looking at we have value rooted and then pass it as toObject() to SetTimeStamp.

Comment on attachment 744198 [details] [diff] [review] Part 3 updated to the changes in part 1 It is a bit ugly that we need to do something like + if type.isDate(): + result = CGGeneric("Date") + if type.nullable(): + result = CGTemplatedType("Nullable", result) in many places. But don't have better suggestions. I hope I didn't miss anything. Still slow at reviewing codegen stuff.

> I'm not sure about the coding style. I'll fix it up. > Couldn't you take JS::Handle<JSObject*> as param. Not easily, as you noted. > And here maybe JS::MutableHandle<Value> vp Also not easily, since the codegen in fact has a Value* it needs to write into. At some point we can fix that, but we'll need to do it for all conversion functions at once... > This could use some comments. Will add some.

> I'll fix it up. So I put braces on own line, but left cx/obj/vp since those are more jsapi-like bits and should look like JS code, imo. Odd style, as you said, but somewhat internally consistent...

> This could use some comments. Actually, I just fixed it to not suck and had khuey review that change.

Comment on attachment 743807 [details] [diff] [review] part 4. Start using the new Date stuff for HTMLInputElement.valueAsDate. Review of attachment 743807 [details] [diff] [review]: ----------------------------------------------------------------- r/sr=me. So good to see that leaving :) ::: dom/interfaces/html/nsIDOMHTMLInputElement.idl @@ -68,5 @@ > attribute DOMString type; > attribute DOMString defaultValue; > attribute DOMString value; > attribute double valueAsNumber; > - [implicit_jscontext] attribute jsval valueAsDate; Could you add a comment saying that valueAsDate isn't present on purpose and why? It will prevent ppl believing that it is not supported by Gecko.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b89e5dc046a6 https://hg.mozilla.org/integration/mozilla-inbound/rev/c4013f62540e https://hg.mozilla.org/integration/mozilla-inbound/rev/3a8c503744ea https://hg.mozilla.org/integration/mozilla-inbound/rev/2f0d5fc274ed

And https://hg.mozilla.org/integration/mozilla-inbound/rev/dffb1419bb19 to fix the unexpected-pass that I thought I'd tested for and clearly failed to...

https://hg.mozilla.org/mozilla-central/rev/b89e5dc046a6 https://hg.mozilla.org/mozilla-central/rev/c4013f62540e https://hg.mozilla.org/mozilla-central/rev/3a8c503744ea https://hg.mozilla.org/mozilla-central/rev/2f0d5fc274ed https://hg.mozilla.org/mozilla-central/rev/dffb1419bb19

Documented at https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Date and https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#Date-struct

Comment on attachment 750860 [details] [diff] [review] Learn how to wrap Dates in constructors r=me, but this should go in a separate bug.