Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Implement the layout for <input type=date>

RESOLVED FIXED in Firefox 53

Status

()

Core
Layout: Form Controls
P1
normal
RESOLVED FIXED
a year ago
4 months ago

People

(Reporter: jessica, Assigned: jessica)

Tracking

(Blocks: 1 bug)

Trunk
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed, relnote-firefox ?)

Details

(Whiteboard: [milestone4])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

a year ago
This bug is about the layout part for date input box. The date picker is handled in Bug 1283385. 

During MozLondon, I asked :jwatt how we should implement the layout pieces for date/time input. The options we had were:
1. Shadow DOM
2. XBL, like videocontrols
3. Pure native anonymous content

For option 1, we asked :wchen about the feasibilty of using shadow dom, but he said shadow dom is not ready to use internally, due to some security concerns. 

After some discussion on implementation, we tend to use XBL, since it'd be easier to handle the complexity of date/time inputs.
Some of the complexity we can think of now are:
- Multiple text boxes (year, month, day, hour, minute, etc) for one input type.
- Focus transferring to the corresponding text box.
- Localization, different ordering for year, month and day. Also, different separators.
- Hiding fields dynamically (milliseconds in time is optional).
- Interaction between picker and the input box.

I am not sure if it's the best way, but I'll try to implement using XBL first, and see if it can satisfy our requirements.

Suggestions are welcome!
(Assignee)

Comment 1

a year ago
One more thing to add is, we believe that this, and other input types, will switch to use shadow dom when it's ready to use internally, since it's pure html technology and more flexible, but until then, we'll try using XBL.
FWIW, I don't know what makes shadow DOM more flexible than XBL. XBL certainly has more features than shadow dom v1. But sure, once shadow DOM v1 is done, we'll need to figure out how to integrate that to native anonymous content and start using it there.
(Assignee)

Comment 3

a year ago
Oh, I thought using shadow DOM, it would be easier for web authors to customize the input control by replacing the shadow content tree. But as you said, we can figure out how to do it once shadow DOM is ready.
Web visible Shadom DOM v1 is explicitly disabled for elements like <input>.
See http://w3c.github.io/webcomponents/spec/shadow/#widl-Element-attachShadow-ShadowRoot-ShadowRootInit-shadowRootInitDict
(Assignee)

Comment 5

a year ago
(In reply to Olli Pettay [:smaug] from comment #4)
> Web visible Shadom DOM v1 is explicitly disabled for elements like <input>.
> See
> http://w3c.github.io/webcomponents/spec/shadow/#widl-Element-attachShadow-
> ShadowRoot-ShadowRootInit-shadowRootInitDict

Oh, right! I remember you mentioned this during MozLondon. Anyway, I'll keep working using XBL for now.
I would think that for either the Shadow DOM or the XBL option, we would be attaching the anonymous content tree (Shadow DOM or XBL) to a native-anonymous element *inside* the input, not to the input itself.  At least, I'd certainly think that's how we would do the XBL option, since I don't think XBL has the security properties we need to attach it directly to an element that the Web page can access.  It seems to me that we *could* do the same with the shadow DOM.  Is there a reason to still prefer XBL over shadow DOM if we were to attach the shadow DOM to native-anonymous content inside the input?

(I'm rather hesitant to add yet more code in a language that we know we'd like to get rid of in the long term.)
Flags: needinfo?(wchen)
Flags: needinfo?(bugs)
(In reply to David Baron :dbaron: ⌚️UTC+8 (review requests must explain patch) from comment #6)
> I would think that for either the Shadow DOM or the XBL option, we would be
> attaching the anonymous content tree (Shadow DOM or XBL) to a
> native-anonymous element *inside* the input, not to the input itself.
Yes, at least for now.

>  At
> least, I'd certainly think that's how we would do the XBL option, since I
> don't think XBL has the security properties we need to attach it directly to
> an element that the Web page can access.  It seems to me that we *could* do
> the same with the shadow DOM.  Is there a reason to still prefer XBL over
> shadow DOM if we were to attach the shadow DOM to native-anonymous content
> inside the input?
We don't have Shadow DOM v1 implemented. v0 implementation will go away.
Use of v0 is equally deprecated to XBL usage, but XBL won't be going away any time soon. v0 will go away once v1 is there.
Flags: needinfo?(bugs)
OK, sounds like XBL is the way to go, then.

(However, I am still encouraging Jessica to use HTML + CSS Flexbox inside the XBL, rather than XUL flexbox.  That is stable and more supported than XUL flexbox.)
Sure, if HTML+CSS can be used, great.

Updated

11 months ago
Priority: -- → P3
(Assignee)

Updated

9 months ago
Assignee: nobody → jjong
(Assignee)

Comment 10

8 months ago
Created attachment 8812717 [details] [diff] [review]
WIP
(Assignee)

Updated

8 months ago
Blocks: 1320225
(Assignee)

Updated

8 months ago
Blocks: 1320227
(Assignee)

Comment 11

8 months ago
Created attachment 8814531 [details] [diff] [review]
patch, v1.
Attachment #8812717 - Attachment is obsolete: true
Whiteboard: [milestone4]
(Assignee)

Comment 12

8 months ago
Comment on attachment 8814531 [details] [diff] [review]
patch, v1.

Review of attachment 8814531 [details] [diff] [review]:
-----------------------------------------------------------------

This patch is to implement the input box for input type=date, so most of the work is in the XBL part. Integration with the date picker will be done in bug 1320225.

Mike, could you take a look at the XBL bits?
I'll ask for Olli's review for DOM part once he is back from vacation.

Thanks. :)
Attachment #8814531 - Flags: review?(mconley)
Comment on attachment 8814531 [details] [diff] [review]
patch, v1.

Review of attachment 8814531 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Jessica,

This looks great! I do think you should have some dom / layout peers look at the work inside dom/html, and layout/[base|forms]. I did review the tests though - it's the native code changes that I'm less sure of.

The tests and the XBL binding changes look okay to me, with some minor notes below (the one that jumps out to me is the leapyear calculation one).

::: accessible/tests/mochitest/jsat/test_output.html
@@ -618,5 @@
>        <label for="input5">Boring label</label><input id="input5" type="checkbox" checked></input>
>        <label for="password">Secret Password</label><input id="password" type="password"></input>
>        <label for="radio_unselected">any old radio button</label><input id="radio_unselected" type="radio"></input>
>        <label for="radio_selected">a unique radio button</label><input id="radio_selected" type="radio" checked></input>
> -      <input id="date" type="date" value="2011-09-29" />

Why is this test being removed? What _is_ going to be uttered when an <input type="date"> is selected?

::: toolkit/content/widgets/datetimebox.xml
@@ +123,5 @@
> +            return;
> +          }
> +
> +          this.log("setFieldsFromInputValue: " + value);
> +          let [year, month, day] = value.split('-');

Might as well be consistent and use double-quotes here.

@@ +139,5 @@
> +        <parameter name="aMonth"/>
> +        <parameter name="aYear"/>
> +        <body>
> +        <![CDATA[
> +          if (aMonth == 2) {

This scares me a little. Maybe leapyears always kinda do.

Can't we use JavaScript's Date() to do this work for us? Example: http://stackoverflow.com/questions/1184334/get-number-days-in-a-specified-month-using-javascript

@@ +193,5 @@
> +            // set to empty.
> +            return;
> +          }
> +
> +          let date = year + "-" + month + "-" + day;

Alternatively:

let date = [year, month, day].join("-");

Although I can't see too much advantage doing it one way over the other.

@@ +230,5 @@
> +            targetField.select();
> +
> +            let n = Number(buffer);
> +            let max = targetField.getAttribute("max");
> +            if (buffer.length >= targetField.maxLength || n * 10 > max) {

Can you explain this a bit more? What's this buffer being used for exactly[1], and where's the magic multiplier "10" coming from?

[1]: Is the buffer to make it so that the field can accept numbers from the keyboard without being interrupted by non-numeric characters?

@@ +327,5 @@
> +
> +      <method name="getCurrentValue">
> +        <body>
> +        <![CDATA[
> +          let year;

Just to be explicit: we're okay with these being undefined if empty?
Attachment #8814531 - Flags: review?(mconley) → review+
Priority: P3 → P1
(Assignee)

Comment 14

7 months ago
(In reply to Mike Conley (:mconley) from comment #13)
> Comment on attachment 8814531 [details] [diff] [review]
> patch, v1.
> 
> Review of attachment 8814531 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hey Jessica,
> 
> This looks great! I do think you should have some dom / layout peers look at
> the work inside dom/html, and layout/[base|forms]. I did review the tests
> though - it's the native code changes that I'm less sure of.

Thanks Mike, I'll ask :samug for the native code changes.

> 
> The tests and the XBL binding changes look okay to me, with some minor notes
> below (the one that jumps out to me is the leapyear calculation one).
> 
> ::: accessible/tests/mochitest/jsat/test_output.html
> @@ -618,5 @@
> >        <label for="input5">Boring label</label><input id="input5" type="checkbox" checked></input>
> >        <label for="password">Secret Password</label><input id="password" type="password"></input>
> >        <label for="radio_unselected">any old radio button</label><input id="radio_unselected" type="radio"></input>
> >        <label for="radio_selected">a unique radio button</label><input id="radio_selected" type="radio" checked></input>
> > -      <input id="date" type="date" value="2011-09-29" />
> 
> Why is this test being removed? What _is_ going to be uttered when an <input
> type="date"> is selected?

type="date" is not longer a text input, so the test fails. Our MVP for type="date" does not include localization nor accessibility, so I'm removing the test for now.

> 
> ::: toolkit/content/widgets/datetimebox.xml
> @@ +123,5 @@
> > +            return;
> > +          }
> > +
> > +          this.log("setFieldsFromInputValue: " + value);
> > +          let [year, month, day] = value.split('-');
> 
> Might as well be consistent and use double-quotes here.

Sure, will do.

> 
> @@ +139,5 @@
> > +        <parameter name="aMonth"/>
> > +        <parameter name="aYear"/>
> > +        <body>
> > +        <![CDATA[
> > +          if (aMonth == 2) {
> 
> This scares me a little. Maybe leapyears always kinda do.
> 
> Can't we use JavaScript's Date() to do this work for us? Example:
> http://stackoverflow.com/questions/1184334/get-number-days-in-a-specified-
> month-using-javascript

Using JavaScript's Date() sounds good to me. 

> 
> @@ +193,5 @@
> > +            // set to empty.
> > +            return;
> > +          }
> > +
> > +          let date = year + "-" + month + "-" + day;
> 
> Alternatively:
> 
> let date = [year, month, day].join("-");
> 
> Although I can't see too much advantage doing it one way over the other.

Using join() looks nicer, thanks. :)

> 
> @@ +230,5 @@
> > +            targetField.select();
> > +
> > +            let n = Number(buffer);
> > +            let max = targetField.getAttribute("max");
> > +            if (buffer.length >= targetField.maxLength || n * 10 > max) {
> 
> Can you explain this a bit more? What's this buffer being used for
> exactly[1], and where's the magic multiplier "10" coming from?
> 
> [1]: Is the buffer to make it so that the field can accept numbers from the
> keyboard without being interrupted by non-numeric characters?

The buffer is to save what is already typed in by the user, it is different from what is shown in the input field. For example, in day field, if user types in "2", the buffer would be "2", but it is shown as "02", then user types in "3", the buffer would be "23", and shown as "23".
When the buffer's length reaches the maxlength, we'll advance to the next input field, also if the user enters a digit that does not allow more digits (this is the "n * 10 > max" part), e.g. "2" in month field or "4" in day field, we'll advance the next field.

> 
> @@ +327,5 @@
> > +
> > +      <method name="getCurrentValue">
> > +        <body>
> > +        <![CDATA[
> > +          let year;
> 
> Just to be explicit: we're okay with these being undefined if empty?

Yes, if the field is undefined, the picker does not update that field.
See Also: → bug 1283385
(Assignee)

Comment 15

7 months ago
Created attachment 8819799 [details] [diff] [review]
patch, v2.

Addressed review comment 13.

Olli, could you take a look at the native code changes? Thanks.
Attachment #8814531 - Attachment is obsolete: true
Attachment #8819799 - Flags: review?(bugs)

Updated

7 months ago
Attachment #8819799 - Flags: review?(bugs) → review+
(Assignee)

Comment 16

7 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9367784fcca3fe10013fa070f9eddb6aacfae437
Keywords: checkin-needed

Comment 17

7 months ago
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5122a14c6fe6
Implement the layout for <input type=date>. r=mconley,smaug
Keywords: checkin-needed

Comment 18

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5122a14c6fe6
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
status-firefox50: affected → ---
Flags: needinfo?(wchen)
Release Note Request (optional, but appreciated)
[Why is this notable]: New cool feature
[Affects Firefox for Android]: Don't know?!
[Suggested wording]: New form element: <input type=date>
[Links (documentation, blog post, etc)]: ?
relnote-firefox: --- → ?
(In reply to Sylvestre Ledru [:sylvestre] from comment #19)
> Release Note Request (optional, but appreciated)
> [Why is this notable]: New cool feature
> [Affects Firefox for Android]: Don't know?!
> [Suggested wording]: New form element: <input type=date>
> [Links (documentation, blog post, etc)]: ?

This feature is still behind the preference dom.forms.datetime, so it is probably too early to request a release note.
Or did I miss something and it is planned to let it ride the trains already?

Sebastian
Flags: needinfo?(jjong)
I haven't added it to the release notes, it is just to keep it on our radar.
I see. Thank you for the fast reply!
Anyway, it would be good to know the current plan about releasing this feature. I assume it will be enabled by default in bug 1283381. Is that correct, Jessica? Or will all input types be implemented first before releasing it, i.e. will users have to wait for bug 888320 to be fixed?

Sebastian
(Assignee)

Comment 23

6 months ago
(In reply to Sebastian Zartner [:sebo] from comment #22)
> I see. Thank you for the fast reply!
> Anyway, it would be good to know the current plan about releasing this
> feature. I assume it will be enabled by default in bug 1283381. Is that
> correct, Jessica? Or will all input types be implemented first before
> releasing it, i.e. will users have to wait for bug 888320 to be fixed?
> 
> Sebastian

Our current plan is to finish implementing input type=date and type=time, including localization and blocker bugs, and ship these two types first, since they are the most used among the five datetime input types. We don't have a specific date for shipping, but if you're interested, you can follow our wiki page: https://wiki.mozilla.org/TPE_DOM/Date_time_input_types#Roadmap
Flags: needinfo?(jjong)
(In reply to Jessica Jong [:jessica] from comment #23)
> Our current plan is to finish implementing input type=date and type=time,
> including localization and blocker bugs, and ship these two types first,
> since they are the most used among the five datetime input types. We don't
> have a specific date for shipping, but if you're interested, you can follow
> our wiki page: https://wiki.mozilla.org/TPE_DOM/Date_time_input_types#Roadmap

Thank you for the info!
FWIW, I've added an entry to the experimental features at https://developer.mozilla.org/en-US/Firefox/Experimental_features#HTML.

Sebastian
(Assignee)

Comment 25

6 months ago
(In reply to Sebastian Zartner [:sebo] from comment #24)
> Thank you for the info!
> FWIW, I've added an entry to the experimental features at
> https://developer.mozilla.org/en-US/Firefox/Experimental_features#HTML.
> 
> Sebastian

Cool, thanks!
Since this is documented on MDN in developer features, I don't think we need a release note. At least, not until we release it as on by default.
(Assignee)

Updated

4 months ago
See Also: → bug 769352
You need to log in before you can comment on or make changes to this bug.