Closed
Bug 874640
Opened 11 years ago
Closed 11 years ago
Make HTMLInputElement.valueAsDate (at least) pref-controlled
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: Waldo, Assigned: mounir)
References
()
Details
Attachments
(1 file)
3.11 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
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.
Assignee | ||
Comment 2•11 years ago
|
||
This should do.
Assignee | ||
Comment 3•11 years ago
|
||
Try: https://tbpl.mozilla.org/?tree=Try&rev=9281e6314874
Comment 4•11 years ago
|
||
Comment on attachment 752802 [details] [diff] [review] Patch r=me
Attachment #752802 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Thanks for the quick review :) https://hg.mozilla.org/integration/mozilla-inbound/rev/8b1edba45296
Flags: in-testsuite+
Target Milestone: --- → mozilla24
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8b1edba45296 https://hg.mozilla.org/mozilla-central/rev/e5047c0ee384
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•