1177510 - HTML input element's max / min attribute does not work on Fennec.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: input_elements_sample.html

The input element's max / min attribute does not work on fennec.

Reproduce step:
1) Open sample page(attach file) on fennec.
2) Tap the datetime element.
3) choose the value over 2015-9-1T23:59:59.

Fennec Actual Result:
- Can choose out of the range value.

Fennec Expected Result:
- Can't choose out of the range value.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Confirmed. This is a spec compliance issue.

Comment 2

[Mantaroh Yoshinaga[:mantaroh]]

When display the DateTimePicker, it set the max / min value.
Min Value is '1/1/1', Max Value is '9999/12/31'.[1]

Probably, It perform correct behavior, If set the min / max attribute value.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/DateTimePicker.java#249

Comment 3

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: WIP_DateTimePicker.patch

I tried the whether or not　work min / max attribute.
Then I confirmed it work fine.

Next step is to implement the code of getting the min / max attribute from contents.

Comment 4

[:Margaret Leibovic]

Thanks for working on this! Let us know if you need any help.

Comment 5

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: bug1177510.html

Add test sample page.

Fennec input element min / max attribute behavior is as follow.
----------------------------------------------
- type=date : can input out of range value. However notified invalid data using red frame.
- type=time : same type=date.
- type=number : same type=date.
- type=range : can't input out of range value.
- type=datetime : can input out of range value.
- type=datetime-local : can input out of range value.
----------------------------------------------

Comment 6

[Mantaroh Yoshinaga[:mantaroh]]

Hi wesj,margaret,

I have a question about behavior of input widget.

Should datetime picker prevent input out of range value?
or Should examine inputted value after setting to value? (Like notifying invalid data using red frame.)
If fixed this bug, the behavior of date / time picker is different from date /time type and datetime / datetime-local type.

best regards.

Comment 7

[Wesley Johnston (:wesj)]

I think it should prevent inputting bad values. That seems better than forcing the user to adjust.

Comment 8

[:Margaret Leibovic]

(In reply to Wesley Johnston (:wesj) from comment #7)
> I think it should prevent inputting bad values. That seems better than
> forcing the user to adjust.

I agree with wesj, I think it would be best to try to prevent that bad input.

Comment 9

[Mantaroh Yoshinaga[:mantaroh]]

wesj, margaret,
Thank you for reply!

I would like to change to prevent inputting invalid value.
However, It isn't using the DateTimePicker when type=time.[1]

It used the TimePicker(Pure Android Widget).
So I would like to separate bug.
(this DateTimePicker bug / TimePicker bug)

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/prompts/PromptInput.java#206

Comment 10

[Wesley Johnston (:wesj)]

Sounds great. Lots of small bugs is always our preference to one big one. Go ahead and split this up as much as you want/need to.

Comment 11

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: bug1177510.patch

Hi wesj,

Created the patch in order to support max/min attribute.
Could you review it?

Comment 12

[Wesley Johnston (:wesj)]

Comment on attachment 8637138 [details] [diff] [review]
bug1177510.patch

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

This looks really good to me! Unfortunately, I don't have an Android device handy for testing. Lets let one of those devs have a look at this too. Margaret.

::: mobile/android/base/prompts/PromptInput.java
@@ +219,5 @@
>                  mView = (View)input;
>              } else if (mType.equals("datetime-local") || mType.equals("datetime")) {
>                  DateTimePicker input = new DateTimePicker(context, "yyyy-MM-dd HH:mm", mValue.replace("T"," ").replace("Z", ""),
> +                                                          DateTimePicker.PickersState.DATETIME, 
> +                                                          mMinValue.replace("T"," ").replace("Z",""), mMaxValue.replace("T"," ").replace("Z", ""));

We should probably make this a helper function to keep all these in sync.

::: mobile/android/base/widget/DateTimePicker.java
@@ +325,5 @@
>          }
>  
> +	// Set the min / max attribute.
> +	try {
> +	    if (minDateValue != null && !minDateValue.equals("")) {

TextUtils.isEmpty(minDateValue)

Comment 13

[Mantaroh Yoshinaga[:mantaroh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ad1f0c9437a

Comment 14

[:Margaret Leibovic]

Can you help explain what I should do to test this locally? I made a build with your patch and visited the test page, but it seems like I can still enter values outside the min/max range. However, the inputs are flagged as invalid (red border) as expected. Is this the expected behavior?

Comment 15

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to :Margaret Leibovic from comment #14)
margaret,

Thank you for reply!

> Can you help explain what I should do to test this locally?
The first input element of attachment 8626289 [details] is setting min="2012-05-12T00:00:00" max="2015-9-1T23:59:59".
Perhaps you can't input out of range value.(e.g. 2012/5/11 or 2015/9/2..etc)

If you can choose out of range value, this patch is not affect this issue..

I confirmed to try result[1].
According to this object, it can't input out of range value.

Could you please confirm applied this patch?

[1] ftp://ftp.mozilla.org/pub/firefox/try-builds/[e-mail]-4ad1f0c9437a/

Comment 16

[:Margaret Leibovic]

I'm sorry I haven't had time to get to this! Maybe kbrosnan can help verify that this patch fixes the problem, although we probably need a new try push at this point.

Otherwise, feel free to go ahead and check this in with wesj's review, and we can verify when it lands on Nightly.

Comment 17

[Pulsebot]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/52a402b0f7bc