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

RESOLVED FIXED in Firefox 43

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mantaroh, Assigned: mantaroh)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 43
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 affected, firefox38.0.5 affected, firefox39 affected, firefox40 affected, firefox41 affected, firefox42 affected, firefox43 fixed, fennec+)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Created attachment 8626289 [details]
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.
Confirmed. This is a spec compliance issue.
tracking-fennec: --- → ?
status-firefox38: --- → affected
status-firefox38.0.5: --- → affected
status-firefox39: --- → affected
status-firefox40: --- → affected
status-firefox41: --- → affected
status-firefox42: --- → affected
OS: Unspecified → Android
Hardware: Unspecified → All
(Assignee)

Comment 2

3 years ago
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
(Assignee)

Comment 3

3 years ago
Created attachment 8628755 [details] [diff] [review]
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.
tracking-fennec: ? → +

Comment 4

3 years ago
Thanks for working on this! Let us know if you need any help.
Assignee: nobody → mantaroh
(Assignee)

Comment 5

3 years ago
Created attachment 8632047 [details]
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.
----------------------------------------------
(Assignee)

Comment 6

3 years ago
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.
Flags: needinfo?(wjohnston)
Flags: needinfo?(margaret.leibovic)
I think it should prevent inputting bad values. That seems better than forcing the user to adjust.
Flags: needinfo?(wjohnston)

Comment 8

3 years ago
(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.
Flags: needinfo?(margaret.leibovic)
(Assignee)

Comment 9

3 years ago
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
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.
(Assignee)

Updated

3 years ago
Blocks: 1184843
(Assignee)

Comment 11

3 years ago
Created attachment 8637138 [details] [diff] [review]
bug1177510.patch

Hi wesj,

Created the patch in order to support max/min attribute.
Could you review it?
Attachment #8628755 - Attachment is obsolete: true
Attachment #8637138 - Flags: review?(wjohnston)
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)
Attachment #8637138 - Flags: review?(wjohnston)
Attachment #8637138 - Flags: review?(margaret.leibovic)
Attachment #8637138 - Flags: review+

Comment 14

3 years ago
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?
Flags: needinfo?(mantaroh)
(Assignee)

Comment 15

3 years ago
(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/mantaroh@gmail.com-4ad1f0c9437a/
Flags: needinfo?(mantaroh) → needinfo?(margaret.leibovic)

Comment 16

3 years ago
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.
Flags: needinfo?(margaret.leibovic)

Updated

3 years ago
Attachment #8637138 - Flags: review?(margaret.leibovic)
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/52a402b0f7bc
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Depends on: 1239823
You need to log in before you can comment on or make changes to this bug.