Upgrade stepUp()/stepDown() implementation to match spec.

Upgrade stepUp()/stepDown() implementation to match spec.

The spec for stepUp()/stepDown()

  http://www.whatwg.org/specs/web-apps/current-work/#dom-input-stepdown

requires that out-of-step values snap to step, just like the UI
implementation will do. Hence, bring the required clamping/snapping
handling into scope.

Also, stepUp()/stepDown() shouldn't throw if the current value is not
a number, nor if the resulting stepped-to value is too far out of
range. Snapping to range (modulo step base) is what's supposed to
happen now.

R=yosin@chromium.org,keishi1@chromium.org,tkent@chromium.org
BUG=339194
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=166525

[sof, 2014-02-04 11:30:59]

When you next have some time, please take a look.

For the test changes, I failed to resist the urge to superficially tidy up a
bit..hope that's ok & not too unreadable. i.e., please just focus on the lines
that no longer use shouldThrow(). Thanks.

[yosin_UTC9, 2014-02-05 01:30:43]

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/date/...
File LayoutTests/fast/forms/date/date-stepup-stepdown.html (right):

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/date/...
LayoutTests/fast/forms/date/date-stepup-stepdown.html:51:
shouldBeEqualToString('stepDown("1970-01-04", "2", null)', '1970-01-03');
I'm not sure why result is "1970-01-03". 2 days before of "1970-01-04" look
"1970-01-02".

[tkent, 2014-02-05 01:46:58]

+keishi

[keishi, 2014-02-05 02:26:35]

lgtm

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/date/...
File LayoutTests/fast/forms/date/date-stepup-stepdown.html (right):

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/date/...
LayoutTests/fast/forms/date/date-stepup-stepdown.html:51:
shouldBeEqualToString('stepDown("1970-01-04", "2", null)', '1970-01-03');
On 2014/02/05 01:30:43, Yoshi wrote:
> I'm not sure why result is "1970-01-03". 2 days before of "1970-01-04" look
> "1970-01-02".

I think it is because 1970-01-04 is a step mismatch. In this patch the value is
snapped-to-step before adding the delta.

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/range...
File LayoutTests/fast/forms/range/range-stepup-stepdown.html (right):

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/range...
LayoutTests/fast/forms/range/range-stepup-stepdown.html:111: // FIXME: these
shouldn't throw according to the spec; commented-out version give its expected
outcomes.
Do we need to keep this?

https://codereview.chromium.org/136783006/diff/1/Source/core/html/forms/Input...
File Source/core/html/forms/InputType.cpp (left):

https://codereview.chromium.org/136783006/diff/1/Source/core/html/forms/Input...
Source/core/html/forms/InputType.cpp:816: const Decimal acceptableErrorValue =
stepRange.acceptableError();
nit: StepRange::acceptableError() can be private now.

[sof, 2014-02-05 07:48:53]

Thanks for going through all these step changes in detail.

(Provided the bots turns green, I'll send this along.)

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/date/...
File LayoutTests/fast/forms/date/date-stepup-stepdown.html (right):

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/date/...
LayoutTests/fast/forms/date/date-stepup-stepdown.html:51:
shouldBeEqualToString('stepDown("1970-01-04", "2", null)', '1970-01-03');
On 2014/02/05 02:26:36, keishi1 wrote:
> On 2014/02/05 01:30:43, Yoshi wrote:
> > I'm not sure why result is "1970-01-03". 2 days before of "1970-01-04" look
> > "1970-01-02".
> 
> I think it is because 1970-01-04 is a step mismatch. In this patch the value
is
> snapped-to-step before adding the delta. 

That matches my interpretation. The step base is 1970-01-01 and the step is 2,
hence 1970-01-04 is therefore a mismatch when calling stepDown(). It snaps to
1970-01-03, and sets value to that.

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/range...
File LayoutTests/fast/forms/range/range-stepup-stepdown.html (right):

https://codereview.chromium.org/136783006/diff/1/LayoutTests/fast/forms/range...
LayoutTests/fast/forms/range/range-stepup-stepdown.html:111: // FIXME: these
shouldn't throw according to the spec; commented-out version give its expected
outcomes.
On 2014/02/05 02:26:36, keishi1 wrote:
> Do we need to keep this?

Sorry, an oversight to leave this behind. Removed now.

https://codereview.chromium.org/136783006/diff/1/Source/core/html/forms/Input...
File Source/core/html/forms/InputType.cpp (left):

https://codereview.chromium.org/136783006/diff/1/Source/core/html/forms/Input...
Source/core/html/forms/InputType.cpp:816: const Decimal acceptableErrorValue =
stepRange.acceptableError();
On 2014/02/05 02:26:36, keishi1 wrote:
> nit: StepRange::acceptableError() can be private now.

Nice, made it so.

[tkent, 2014-02-05 08:33:23]

https://codereview.chromium.org/136783006/diff/120001/Source/core/html/forms/...
File Source/core/html/forms/InputType.cpp (right):

https://codereview.chromium.org/136783006/diff/120001/Source/core/html/forms/...
Source/core/html/forms/InputType.cpp:833: applyStep(newValue, count - 1,
AnyIsDefaultStep, DispatchChangeEvent, IGNORE_EXCEPTION);
Please add EventQueueScope in this function.
setValueAsDecimal can remove this InputType object and/or HTMLInputElement.

[sof, 2014-02-05 08:42:50]

https://codereview.chromium.org/136783006/diff/120001/Source/core/html/forms/...
File Source/core/html/forms/InputType.cpp (right):

https://codereview.chromium.org/136783006/diff/120001/Source/core/html/forms/...
Source/core/html/forms/InputType.cpp:833: applyStep(newValue, count - 1,
AnyIsDefaultStep, DispatchChangeEvent, IGNORE_EXCEPTION);
On 2014/02/05 08:33:24, tkent wrote:
> Please add EventQueueScope in this function.
> setValueAsDecimal can remove this InputType object and/or HTMLInputElement.

Thanks, done.

[tkent, 2014-02-05 08:48:37]

lgtm

[sof, 2014-02-05 09:05:31]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-02-05 09:05:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sigbjornf@opera.com/136783006/190001

[commit-bot: I haz the power, 2014-02-05 12:10:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-05 12:11:02]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[sof, 2014-02-05 12:12:41]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-02-05 12:12:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sigbjornf@opera.com/136783006/190001

[commit-bot: I haz the power, 2014-02-05 14:09:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-05 14:09:21]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[sof, 2014-02-05 17:27:19]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-02-05 17:27:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sigbjornf@opera.com/136783006/190001

[commit-bot: I haz the power, 2014-02-05 18:21:15]

Change committed as 166525