[Webkit-unassigned] [Bug 45491] when empty, clicking "down" on outer-spin-button returns "max value"
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Nov 25 17:00:23 PST 2010
https://bugs.webkit.org/show_bug.cgi?id=45491
Kent Tamura <tkent at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #74839|review? |review-
Flag| |
--- Comment #22 from Kent Tamura <tkent at chromium.org> 2010-11-25 17:00:23 PST ---
(From update of attachment 74839)
View in context: https://bugs.webkit.org/attachment.cgi?id=74839&action=review
I stopped reviewing halfway. The test quality is bad. Please be careful.
> JavaScriptCore/ChangeLog:20
> + Another option was to provide another currentTime() to return the current local milliseconds.
> + Surveying the codes, however, all of existing "milliseconds" in double variables are
> + consistently in UTC. I guess we should not emit "milliseconds" in non-UTC in the code.
> + Finally I locked "milliseconds" in non-UTC in calculating default values.
This paragraph looks unnecessary.
Also, I feel it's strange to have the word "I" in ChangeLog.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:1
> +description('Check stepUp() and stepDown() bahevior from renderer for type=date, datetime, datetime-local, month, time, week.');
The test contains "number" and "range" types.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:88
> +debug('Non-number arguments');
> +shouldBe('stepUp("2010-02-10", null, null, "0")', '"2010-02-10"');
> +shouldBe('stepDown("2010-02-10", null, null, "0")', '"2010-02-10"');
> +shouldBe('stepUp("2010-02-10", null, null, "foo")', '"2010-02-10"');
> +shouldBe('stepDown("2010-02-10", null, null, "foo")', '"2010-02-10"');
> +shouldBe('stepUp("2010-02-10", null, null, null)', '"2010-02-10"');
> +shouldBe('stepDown("2010-02-10", null, null, null)', '"2010-02-10"');
The 4th parameter is recognized in JavaScript code and is not passed to C++ code.
We don't need to test invalid values.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:101
> +shouldBe('stepUp("2010-02-10", "3.40282346e+38", null)','"1970-01-01"');
> +shouldBe('stepDown("2010-02-10", "3.40282346e+38", null)', '"275760-09-13"');
These results look very strange. Are they expected?
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:114
> +debug('Non-number arguments');
> +shouldBe('stepUp("2010-02-10T20:13Z", null, null, "0")', '"2010-02-10T20:13Z"');
> +shouldBe('stepDown("2010-02-10T20:13Z", null, null, "0")', '"2010-02-10T20:13Z"');
> +shouldBe('stepUp("2010-02-10T20:13Z", null, null, "foo")', '"2010-02-10T20:13Z"');
> +shouldBe('stepDown("2010-02-10T20:13Z", null, null, "foo")', '"2010-02-10T20:13Z"');
> +shouldBe('stepUp("2010-02-10T20:13Z", null, null, null)', '"2010-02-10T20:13Z"');
> +shouldBe('stepDown("2010-02-10T20:13Z", null, null, null)', '"2010-02-10T20:13Z"');
ditto.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:140
> +debug('Non-number arguments');
> +shouldBe('stepUp("2010-02-10T20:13", null, null, "0")', '"2010-02-10T20:13"');
> +shouldBe('stepDown("2010-02-10T20:13", null, null, "0")', '"2010-02-10T20:13"');
> +shouldBe('stepUp("2010-02-10T20:13", null, null, "foo")', '"2010-02-10T20:13"');
> +shouldBe('stepDown("2010-02-10T20:13", null, null, "foo")', '"2010-02-10T20:13"');
> +shouldBe('stepUp("2010-02-10T20:13", null, null, null)', '"2010-02-10T20:13"');
> +shouldBe('stepDown("2010-02-10T20:13", null, null, null)', '"2010-02-10T20:13"');
ditto.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:166
> +debug('Non-number arguments');
> +shouldBe('stepUp("2010-02", null, null, "0")', '"2010-02"');
> +shouldBe('stepDown("2010-02", null, null, "0")', '"2010-02"');
> +shouldBe('stepUp("2010-02", null, null, "foo")', '"2010-02"');
> +shouldBe('stepDown("2010-02", null, null, "foo")', '"2010-02"');
> +shouldBe('stepUp("2010-02", null, null, null)', '"2010-02"');
> +shouldBe('stepDown("2010-02", null, null, null)', '"2010-02"');
ditto.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:195
> +debug('Non-number arguments');
> +shouldBe('stepUp("0", null, null, "0")', '"0"');
> +shouldBe('stepDown("0", null, null, "0")', '"0"');
> +shouldBe('stepUp("0", null, null, "foo")', '"0"');
> +shouldBe('stepDown("0", null, null, "foo")', '"0"');
> +shouldBe('stepUp("0", null, null, null)', '"0"');
> +shouldBe('stepDown("0", null, null, null)', '"0"');
ditto.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:205
> +debug('Extra arguments');
> +shouldBe('input.value = "0"; input.min = null; input.step = null; input.stepUp(1, 2); input.value', '"1"');
> +shouldBe('input.value = "1"; input.stepDown(1, 3); input.value', '"0"');
These are not related to the code change.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:212
> +debug('Invalid step value');
> +shouldBe('stepUp("0", "foo", null)', '"1"');
> +shouldBe('stepUp("1", "0", null)', '"2"');
> +shouldBe('stepUp("2", "-1", null)', '"3"');
> +debug('Step=any');
> +shouldBe('stepUp("0", "any", null)', '"0"');
> +shouldBe('stepDown("0", "any", null)', '"0"');
We should add test cases with empty/invalid values and invalid/any step.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:217
> +shouldBe('input.value', '"0"');
This doesn't test anything.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:222
> +shouldBe('input.value', '"0"');
ditto.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:224
> +debug('stepDown()/stepUp() for stepMismatch values');
Other types should have similar test cases.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:226
> +shouldBe('input.stepDown(); input.value', '"0"');
This is not related to the code change.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:227
> +shouldBe('input.min = "0"; stepUp("9", "10", "", 9)', '"90"');
This looks equivalent to
shouldBe('input.min = "0"; stepUp("9", "10", "")', '"10"');
The 4th parameter is not needed.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:236
> +shouldBe('input.stepUp(); input.stepUp(); input.stepUp(); input.stepUp(); input.stepUp(); input.stepUp(); input.stepUp(); input.stepUp(); input.stepUp(); input.stepUp(); input.value', '"3"');
This is not related to the code change.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:238
> +shouldBe('for (var i = 0; i < 255; i++) { input.stepDown(); }; input.value', '"0"');
ditto.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:250
> +debug('function arguments are (min, max, step, value, [stepCount])');
Nice to have explanation of stepUp() and stepDown() too.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:260
> +debug('Non-number arguments (stepCount)');
> +shouldBe('stepUpExplicitBounds(null, null, null, "0", "0")', '"0"');
> +shouldBe('stepDownExplicitBounds(null, null, null, "0", "0")', '"0"');
> +shouldBe('stepUpExplicitBounds(null, null, null, "0", "foo")', '"0"');
> +shouldBe('stepDownExplicitBounds(null, null, null, "0", "foo")', '"0"');
> +shouldBe('stepUpExplicitBounds(null, null, null, "0", null)', '"0"');
> +shouldBe('stepDownExplicitBounds(null, null, null, "0", null)', '"0"');
It makes no sense to test invalid 5th parameter.
> LayoutTests/fast/forms/script-tests/input-stepup-stepdown-from-renderer.js:270
> +debug('Extra arguments');
> +shouldBe('setInputAttributes(null, null, null, "0"); input.stepUp(1,2); input.value', '"1"');
> +shouldBe('setInputAttributes(null, null, null, "1"); input.stepDown(1,3); input.value', '"0"');
These are not related to the code change.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list