Bug 106983 – INPUT_MULTIPLE_FIELDS_UI: Unable to edit a time field with step=86400, and confusing appearance

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Kent Tamura]

http://code.google.com/p/chromium/issues/detail?id=168979

What steps will reproduce the problem?
1. Open data:text/html,<input type=time min="17:00" step=86400 id=time1><script>alert("Value: "+time1.value)</script>

What is the expected output? What do you see instead?
Expected:
 - Users can clear the initial value because the time field has no "required" attribute
 - HTMLInputElement::value should return what we see. In this case, "17:00" should be returned.

Actual:
 - There are no ways to clear the initial value
 - HTMLInputElement::value is empty

[Kent Tamura]

Created attachment 182928 [details]
Patch

[Kentaro Hara]

Comment on attachment 182928 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182928&action=review

> Source/WebCore/ChangeLog:14
> +        - We don't make hour fields read-only for input[type=time].

Shall we add a test case for input[type=time] too? BTW, what about input[type=datetime]?

> Source/WebCore/ChangeLog:36
> +        are read-only. Note that we don't need to check read-only status of year
> +        and month fields explicitly here because a day field can be read-only

Shall we add an ASSERT() to check this?

[WebKit Review Bot]

Comment on attachment 182928 [details]
Patch

Attachment 182928 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15905302

New failing tests:
fast/forms/time/time-appearance-basic.html

[Kent Tamura]

Comment on attachment 182928 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182928&action=review

>> Source/WebCore/ChangeLog:14
>> +        - We don't make hour fields read-only for input[type=time].
> 
> Shall we add a test case for input[type=time] too? BTW, what about input[type=datetime]?

Yes, I already added tests for input[type=time] and added a reason of no input[type=datetime] tests to LayoutTests/ChangeLog.

>> Source/WebCore/ChangeLog:36
>> +        and month fields explicitly here because a day field can be read-only
> 
> Shall we add an ASSERT() to check this?

will do.

[Kentaro Hara]

Comment on attachment 182928 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182928&action=review

Looks good to me.

>>> Source/WebCore/ChangeLog:14
>>> +        - We don't make hour fields read-only for input[type=time].
>> 
>> Shall we add a test case for input[type=time] too? BTW, what about input[type=datetime]?
> 
> Yes, I already added tests for input[type=time] and added a reason of no input[type=datetime] tests to LayoutTests/ChangeLog.

Sorry, I missed that.

[Kent Tamura]

Created attachment 182937 [details]
Patch 2

Add assertions, handle time-appearance-basic.html

[WebKit Review Bot]

Comment on attachment 182937 [details]
Patch 2

Clearing flags on attachment: 182937

Committed r139865: <http://trac.webkit.org/changeset/139865>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Levi Weintraub]

Comment on attachment 182937 [details]
Patch 2

View in context: https://bugs.webkit.org/attachment.cgi?id=182937&action=review

> LayoutTests/platform/chromium/TestExpectations:4094
> +# Need rebaseline
> +webkit.org/b/106983 fast/forms/time/time-appearance-basic.html [ Failure ]

For future reference, "Failure" sadly doesn't encompass "ImageOnlyFailure", so this actually made the bots red :( I rebaselined in r139892.

[Kent Tamura]

(In reply to comment #9)
> > +webkit.org/b/106983 fast/forms/time/time-appearance-basic.html [ Failure ]
> 
> For future reference, "Failure" sadly doesn't encompass "ImageOnlyFailure", so this actually made the bots red :( I rebaselined in r139892.

oh, I'm sorry for that, and thank you!