Bug 98227 - DateTimeYearFieldElement should respect min/max values specified by page authors
Summary: DateTimeYearFieldElement should respect min/max values specified by page authors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on: 98228 98230
Blocks: 97299 97997 98226
  Show dependency treegraph
 
Reported: 2012-10-02 20:48 PDT by Kent Tamura
Modified: 2012-10-03 21:32 PDT (History)
4 users (show)

See Also:


Attachments
Patch (21.32 KB, patch)
2012-10-03 00:31 PDT, Kent Tamura
no flags Details | Formatted Diff | Diff
Patch 2 (26.77 KB, patch)
2012-10-03 04:19 PDT, Kent Tamura
morrita: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2012-10-02 20:48:11 PDT
DateTimeYearFieldElement should respect min/max values specified by page authors.
Comment 1 Kent Tamura 2012-10-03 00:31:29 PDT
Created attachment 166819 [details]
Patch
Comment 2 WebKit Review Bot 2012-10-03 02:28:29 PDT
Comment on attachment 166819 [details]
Patch

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

New failing tests:
fast/forms/month-multiple-fields/month-multiple-fields-appearance-pseudo-classes.html
fast/forms/week-multiple-fields/week-multiple-fields-appearance-basic.html
fast/forms/week-multiple-fields/week-multiple-fields-appearance-pseudo-classes.html
fast/forms/week-multiple-fields/week-multiple-fields-appearance-style.html
fast/forms/week-multiple-fields/week-multiple-fields-appearance-pseudo-elements.html
Comment 3 yosin 2012-10-03 03:03:01 PDT
Comment on attachment 166819 [details]
Patch

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

> Source/WebCore/html/shadow/DateTimeFieldElements.cpp:456
> +    return range().isInRange(fullYear) ? fullYear : DateTimeNumericFieldElement::defaultValueForStepDown();

In current UI document, we use current year if we author doesn't specify "min" attribute, otherwise we use minimum value.
If you think using current year if it is in-range is better, please update UI document.
Comment 4 Kent Tamura 2012-10-03 03:12:13 PDT
(In reply to comment #3)
> In current UI document, we use current year if we author doesn't specify "min" attribute, otherwise we use minimum value.
> If you think using current year if it is in-range is better, please update UI document.

I see. I'll update the implementation.
Comment 5 Kent Tamura 2012-10-03 04:19:01 PDT
Created attachment 166854 [details]
Patch 2
Comment 6 Hajime Morrita 2012-10-03 20:33:26 PDT
Comment on attachment 166854 [details]
Patch 2

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

> Source/WebCore/html/shadow/DateTimeEditElement.cpp:164
> +            yearParams.minIsSpecified = false;

Nit: could be
 yearParams.minIsSpecified = m_parameters.minimumYear != m_parameters.undefinedYear();

> Source/WebCore/html/shadow/DateTimeFieldElements.h:177
> +            : minimumYear(-1)

Why not use undefinedYear()?
Comment 7 Kent Tamura 2012-10-03 20:38:55 PDT
Comment on attachment 166854 [details]
Patch 2

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

>> Source/WebCore/html/shadow/DateTimeFieldElements.h:177
>> +            : minimumYear(-1)
> 
> Why not use undefinedYear()?

Because we'd like to avoid dependency from DateTime*FieldElement to DateTimeEditElement
Comment 8 WebKit Review Bot 2012-10-03 20:43:42 PDT
Comment on attachment 166854 [details]
Patch 2

Rejecting attachment 166854 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
/WebCore/html/shadow/DateTimeNumericFieldElement.h
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-keyboard-events-expected.txt
patching file LayoutTests/fast/forms/month-multiple-fields/month-multiple-fields-keyboard-events.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Hajime Mor..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/14129925
Comment 9 Kent Tamura 2012-10-03 21:32:41 PDT
Committed r130362: <http://trac.webkit.org/changeset/130362>