Bug 94664 - [Forms] Step mismatched value should be editable in multiple field time UI.
Summary: [Forms] Step mismatched value should be editable in multiple field time UI.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on: 94898
Blocks:
  Show dependency tree / graph
 
Reported: 2012-08-21 18:33 PDT by yosin
Modified: 2012-08-23 22:27 PDT (History)
5 users (show)

See Also:

Attachments
Patch 1 (117.34 KB, patch)
2012-08-21 23:22 PDT, yosin 		no flags Details | Formatted Diff | Diff
Patch 2 (119.77 KB, patch)
2012-08-23 21:04 PDT, yosin 		no flags Details | Formatted Diff | Diff
Patch 3 (119.77 KB, patch)
2012-08-23 21:16 PDT, yosin 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-08-21 18:33:57 PDT 
When web author writes <input type=time step=60 value="12:34:56"> in their page, users can't set second field to zero.
Comment 1 yosin 2012-08-21 23:22:19 PDT 
Created attachment 159866 [details]
Patch 1
Comment 2 yosin 2012-08-21 23:23:01 PDT 
Comment on attachment 159866 [details]
Patch 1

Could you review this patch?
Thanks in advance.
Comment 3 Kent Tamura 2012-08-23 18:59:30 PDT 
Comment on attachment 159866 [details]
Patch 1

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

The code change looks ok.
We need another test:
  1. Render <input type=time step=60>
  2. Sets value as "23:59:59.999" by JavaScript
  3. Confirms that additional sub-fields are added to the input.

> Source/WebCore/ChangeLog:8
> +        This patch changeds layout method of multiple field time input UI to

changeds -> changes
Comment 4 yosin 2012-08-23 21:04:55 PDT 
Created attachment 160321 [details]
Patch 2
Comment 5 yosin 2012-08-23 21:06:48 PDT 
Comment on attachment 160321 [details]
Patch 2

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* Add a test file: fast/forms/time-multiple-fields/time-multiple-fields-change-type-on-focus.html
* Update ChangeLog for new test file and fix typo
Comment 6 Kent Tamura 2012-08-23 21:13:47 PDT 
Comment on attachment 160321 [details]
Patch 2

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

> Source/WebCore/ChangeLog:12
> +        Tests: fast/forms/time-multiple-fields/time-multiple-fields-change-type-on-focus.html: for changing layout by value change.

The file name looks incorrect.
It should be time-multiple-fields-change-layout-by-value.html.

> LayoutTests/ChangeLog:20
> +        * fast/forms/time-multiple-fields/time-multiple-fields-change-type-on-focus-expected.txt: Added.
> +        * fast/forms/time-multiple-fields/time-multiple-fields-change-type-on-focus.html: Added for dynamic layout change from value change.

File names look incorrect.
Comment 7 yosin 2012-08-23 21:16:48 PDT 
Created attachment 160322 [details]
Patch 3
Comment 8 yosin 2012-08-23 21:17:58 PDT 
Comment on attachment 160322 [details]
Patch 3

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* Fix wrong new test file name in ChangeLog files
Comment 9 Kent Tamura 2012-08-23 21:20:28 PDT 
Comment on attachment 160322 [details]
Patch 3

ok
Comment 10 yosin 2012-08-23 21:22:45 PDT 
Comment on attachment 160322 [details]
Patch 3

Clearing flags on attachment: 160322

Committed r126534: <http://trac.webkit.org/changeset/126534>
Comment 11 yosin 2012-08-23 21:22:50 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 12 Dominic Cooney 2012-08-23 22:27:22 PDT 
This caused a regression or needs rebaselining on Win 7, see bug 94898.