1120108 - datepicker for input with type=datetime-local does not initialize with the current value on Firefox for Android

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Rémi E.]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0
Build ID: 20141127111021

Steps to reproduce:

I'm using a webpage with input elements with type="datetime-local", they have some value attribute set to something like 2004-08-31T10:22.
Then I click on the field.



Actual results:

The current date is set in the datetime chooser. 



Expected results:

The datetime from the value attribute should be shown.

Comment 1

[Rémi E.]

Additional info:

The behavior can be tested on the following page http://olav.dk/wf2/demo/datetime.asp scrolling down to the datetime-local example.

When a date is selected in the chooser, the field gets a value of the form "2015-01-10 18:54". It contains a space instead of a T.
http://www.w3.org/TR/html-markup/input.datetime-local.html#input.datetime-local.attrs.value

Side note: The behavior in firefoxOS (ZTE OpenC) is correct.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Desktop Firefox doesn't support <input type="datetime-local"> at the moment.  That is, it's treated exactly like <input type="text">.

In both Firefox 34 and current nightly builds in a default configuration on Mac, the value shown on http://olav.dk/wf2/demo/datetime.asp matches the value attribute of the input.  Clicking on the inputs doesn't show a datetime chooser, as expected.

What exact steps are you using to reproduce this?  What preferences did you change from their default values, and to what values did you set them?

Comment 3

[Rémi E.]

Sorry for the misunderstanding.
The report was actually filed against "Firefox for Android" but it seems it has been routed to "DOM, Core". It might need rerouting.
Cheers.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Ah, I see.  So this is really about the Firefox for Android UI, not anything in the DOM, ok.

Comment 5

[:Margaret Leibovic]

wesj, do you know about our plans to implement this?

Comment 6

[Wesley Johnston (:wesj)]

Its interesting that the DOM doesn't reject an invalid string here. I guess eventually we can hand the platform something real and it will fix it up for us. Until then, these are hardcoded here:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/prompts/PromptInput.java#257

Comment 7

[Rémi E.]

Indeed, it seems to be the proper place for the fix. The date format won't accept a 'T' in it so one should probably ".replace()" the space by a 'T'.

I think the parsing side needs also a fix:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/prompts/PromptInput.java#219

(the code block might need to be duplicated if one want to keep the "datetime" (vs "datetime-local" behavior))

I'd have like to propose a patch, but I don't have an android sdk anymore and not much time to install it, sorry.

Comment 8

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: 1120108.patch

Hi,
I create the patch for this bugs.
I think that should modify like comment #5.

I modified sources and confirmed that display correct date when tap datetime-local element on My N5.
(Test page:http://jsbin.com/fijiqi)

Comment 9

[Rémi E.]

Hi and thanks,

This looks good for the display side.

I think that line 257, link above, and maybe 262 need to be updated so that when the user picks a value, the field actually change.

Comment 10

[Mantaroh Yoshinaga[:mantaroh]]

Hi Rémi.

Thank you for confirm this patch.

> I think that line 257, link above, and maybe 262 need to be updated so that
> when the user picks a value, the field actually change.
I understand now!
Let me make sure I understand you correctly.
You mean datetime-local's value should include literal 'T'. (like yyyy-MM-dd'T'HH-mm)

about line 262, datetime returned GMT Time.
So, I think that Fennec should return yyyy-MM-dd'T'HH-mm'Z'.

How do you think about datetime elements?

FYI:
Chrome Mobile and Mobile Safari are not appeard datepicker when datetime input element clicked.

Comment 11

[Rémi E.]

I think we're on the same page, the value should include the 'T' if I read [1] properly.

About the "datetime", I don't know how much it is used. I guess from [2] it should allow to select a timezone also (but that requires more work, maybe it needs to be filed in some sort of feature request). Adding the 'T' and 'Z' for it seems to be a decent fallback behavior, improving over the current state and requiring much less work. An alternative would also be to just consider it as a plain text field as it seems to be done in chrome and safari (c.f. your comment).

Cheers.

[1] http://www.w3.org/TR/html-markup/input.datetime-local.html
[2] http://www.w3.org/TR/html-markup/input.datetime.html

Comment 12

[:Margaret Leibovic]

(In reply to mantaroh from comment #8)
> Created attachment 8575118 [details] [diff] [review]
> 1120108.patch
> 
> Hi,
> I create the patch for this bugs.
> I think that should modify like comment #5.
> 
> I modified sources and confirmed that display correct date when tap
> datetime-local element on My N5.
> (Test page:http://jsbin.com/fijiqi)

I'm not familiar with the date time picker, so I don't feel confident reviewing this patch without spending some time learning about this code.

It looks like mounir reviewed the patch where wesj last touched this logic (bug 888949), so maybe he can take a look at this before wesj comes back from parental leave (I believe that should be in about 2 weeks).

Comment 13

[:Margaret Leibovic]

Comment on attachment 8575118 [details] [diff] [review]
1120108.patch

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

wesj is back from leave, I'm just going to redirect this to him.

Comment 14

[Rémi E.]

I think the patch needs to be complemented before review/merge (see comments 9 to 11).

Comment 15

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: 1120108.patch

I'm sorry late reply.

I modified the patch.
(add literal 'T' on getValue() method.)

and, I will split datetime problem(datetime picker is not include literal 'T' and 'Z')

Thanks

Comment 16

[Wesley Johnston (:wesj)]

Comment on attachment 8582875 [details] [diff] [review]
1120108.patch

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

Looks good to me!

Comment 17

[Mantaroh Yoshinaga[:mantaroh]]

Hi wesj.
This bug is fixed. 
So could you add the checkin-needed flag?

best regards,

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/aa1c8fb66a4e

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/aa1c8fb66a4e