874640 - Make HTMLInputElement.valueAsDate (at least) pref-controlled

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jeff Walden [:Waldo]]

If <input type="date"> is supported, valueAsDate exposes the value as a Date object and allows it to be set.  If it's not supported, the property shouldn't be present at all.  Currently the setter is implemented this way:

void
HTMLInputElement::SetValueAsDate(Nullable<Date> aDate, ErrorResult& aRv)
{
  if (mType != NS_FORM_INPUT_DATE && mType != NS_FORM_INPUT_TIME) {
    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
    return;
  }

which has the result that if type="date" support isn't turned on (dom.experimental_forms), setting the property throws an inscrutable InvalidStateError.  That's not cool.

I have no idea if any other <input> properties should be similarly conditioned; probably all the <input> properties should be audited for proper pref-controlled behavior.  The resulting patches should probably also be backported, as having properties not be pref-controlled can easily break feature detection code.

Comment 1

[Mounir Lamouri (:mounir)]

There is also .valueAsNumber, .min, .max and .step but those are used for type='number', type='range' and date/time types and type='range'. We could PrefControl'd them by checking multiple prefs but type='range' is on by default now so this would be a waste of time I think.

Philosophically I disagree with what has been said in comment 1: .valueAsDate isn't exposed only if <input type='date'> is supported. Content assuming that would be wrong and I doubt there are a lot of them doing so because I believe we are shipping Firefox with .valueAsDate since nearly a year.
However, given how easy it is to make this property pref-conditioned with WebIDL, it's probably worth doing.

Comment 2

[Mounir Lamouri (:mounir)]

Attachment: Patch

This should do.

Comment 3

[Mounir Lamouri (:mounir)]

Try: https://tbpl.mozilla.org/?tree=Try&rev=9281e6314874

Comment 4

[Boris Zbarsky [:bzbarsky]]

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

r=me

Comment 5

[Mounir Lamouri (:mounir)]

Thanks for the quick review :)
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b1edba45296

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/8b1edba45296
https://hg.mozilla.org/mozilla-central/rev/e5047c0ee384