Mounir Lamouri (:mounir) Assignee
	Description • 12 years ago

It should set the value to the empty string if the passed value is null or a Date with NaN value. Otherwise, it should throw a TypeError, per WebIDL spec.

	Mounir Lamouri (:mounir) Assignee
	Comment 1 • 12 years ago

(I forgot to refresh the patch...)

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 12 years ago

Comment on attachment 697485 [details] [diff] [review]
Patch

>+  if (aDate.isNull()) {

isNullOrUndefined(), please.

>+    ok(caught, "Setting " + value + " to .valueAsDate should throw");

Either s/Setting/Assigning/ or "setting .valueAsDate to " + value + " should throw".

r=me with those nits.

	Mounir Lamouri (:mounir) Assignee
	Comment 3 • 12 years ago

(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 697485 [details] [diff] [review]
> Patch
> 
> >+  if (aDate.isNull()) {
> 
> isNullOrUndefined(), please.

The HTML specs say:
"if the new value is null or a Date object representing the NaN time value, then set the value of the element to the empty string".
As far as I understand it, that means "undefined" should be considered as a TypeError. Or did I missed something?

I've added a test for this to make it clearer that undefined should throw.

	Boris Zbarsky [:bzbarsky]
	Comment 4 • 12 years ago

> "if the new value is null 

That's the IDL value, not the JS value.  Nullable IDL values turn both JS null and JS undefined into null IDL values.  If you were actually using our WebIDL bindings, this would happen automatically before you ever got into this code.

Specifically, see http://dev.w3.org/2006/webapi/WebIDL/#es-nullable-type step 3 under "An ECMAScript value V is converted to an IDL nullable type T? value (where T is the inner type) as follows".

	Mounir Lamouri (:mounir) Assignee
	Comment 5 • 12 years ago

Fixed. Thanks for the explanations :)

	Mounir Lamouri (:mounir) Assignee
	Comment 6 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/872a0a08ecce

	Ed Morley [:emorley]
	Comment 7 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/872a0a08ecce

	Mounir Lamouri (:mounir) Assignee
	Comment 8 • 12 years ago

Comment on attachment 697485 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 769370
User impact if declined: adding a new feature with a known bug
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low, we could introduce another bug in the feature by fixing that one but we have a pretty good test coverage
String or UUID changes made by this patch: none

	bhavana bajaj [:bajaj]
	Comment 9 • 12 years ago

Comment on attachment 697485 [details] [diff] [review]
Patch

Low risk uplift for a new feature in Fx20.Approving on aurora.

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 12 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/da2099689dae

	Florian Scholz (Open Web Docs)
	Comment 11 • 11 years ago

See bug 866440 for documentation.