Closed Bug 1120108 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox40 fixed)

RESOLVED FIXED
Firefox 40
Tracking Status
firefox40 --- fixed

People

(Reporter: persona, Assigned: mantaroh)

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Component: General → DOM
Product: Firefox for Android → Core
Version: Firefox 36 → Trunk
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?
Flags: needinfo?(persona)
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.
Flags: needinfo?(persona)
Ah, I see.  So this is really about the Firefox for Android UI, not anything in the DOM, ok.
Component: DOM → General
Product: Core → Firefox for Android
Summary: input with type=datetime-local do not initialize with the current value → datepicker for input with type=datetime-local does not initialize with the current value on Firefox for Android
wesj, do you know about our plans to implement this?
Flags: needinfo?(wjohnston)
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
Flags: needinfo?(wjohnston)
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.
Attached patch 1120108.patch (obsolete) — Splinter Review
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)
Flags: needinfo?(margaret.leibovic)
Attachment #8575118 - Flags: review?(margaret.leibovic)
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.
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.
Flags: needinfo?(persona)
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
(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).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(margaret.leibovic) → needinfo?(mounir)
Assignee: nobody → mantaroh
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.
Attachment #8575118 - Flags: review?(margaret.leibovic) → review?(wjohnston)
I think the patch needs to be complemented before review/merge (see comments 9 to 11).
Flags: needinfo?(persona)
Attached patch 1120108.patchSplinter Review
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
Flags: needinfo?(wjohnston)
Attachment #8582875 - Flags: review?(wjohnston)
Attachment #8575118 - Attachment is obsolete: true
Attachment #8575118 - Flags: review?(wjohnston)
Comment on attachment 8582875 [details] [diff] [review]
1120108.patch

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

Looks good to me!
Attachment #8582875 - Flags: review?(wjohnston) → review+
Flags: needinfo?(wjohnston)
Hi wesj.
This bug is fixed. 
So could you add the checkin-needed flag?

best regards,
Flags: needinfo?(mounir) → needinfo?(wjohnston)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aa1c8fb66a4e
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Flags: needinfo?(wjohnston)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.