Bug 88208

Summary:

REGRESSION(r117929) [Forms] input type=number thinks 0 is not a number

Product:

WebKit

Reporter:

yosin

Component:

Forms

Assignee:

yosin

Status:

RESOLVED FIXED

Severity:

Normal

CC:

tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

URL:

data:text/html,<input type=number min=0 max=50 step=1>

Attachments:

Description	Flags
Patch 1	none
Patch 2	none
Patch 3	none
Patch 4	none
Archive of layout-test-results from ec2-cq-02	none

yosin

Reported 2012-06-03 20:52:16 PDT

Import from http://crbug.com/130910 This is regression of r117929, it seems he didn't change following code: bool NumberInputType::isAcceptableValue(const String& proposedValue) { String standardValue = convertFromVisibleValue(proposedValue); return standardValue.isEmpty() || parseToDoubleForNumberType(standardValue, 0); // Forgot to change } Before r117929, parseToDoubleForNumberType returns bool which indicates true is valid number or not. After r117929, the return value is number or NaN. So, value 0 means false in this case == not acceptable value.

Attachments
Patch 1 (4.87 KB, patch) 2012-06-03 21:49 PDT, yosin	no flags	Details Formatted Diff Diff
Patch 2 (6.06 KB, patch) 2012-06-03 22:36 PDT, yosin	no flags	Details Formatted Diff Diff
Patch 3 (6.07 KB, patch) 2012-06-03 22:49 PDT, yosin	no flags	Details Formatted Diff Diff
Patch 4 (6.02 KB, patch) 2012-06-03 22:52 PDT, yosin	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ec2-cq-02 (616.83 KB, application/zip) 2012-06-03 23:49 PDT, WebKit Review Bot	no flags	Details
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

yosin

Comment 1 2012-06-03 21:49:47 PDT

Created attachment 145514 [details] Patch 1

Kent Tamura

Comment 2 2012-06-03 21:58:21 PDT

Comment on attachment 145514 [details] Patch 1 View in context: https://bugs.webkit.org/attachment.cgi?id=145514&action=review > Source/WebCore/html/NumberInputType.cpp:255 > - return standardValue.isEmpty() || parseToDoubleForNumberType(standardValue, 0); > + return standardValue.isEmpty() || isfinite(parseToDoubleForNumberType(standardValue)); Did you confirm we had no other oversights? > LayoutTests/ChangeLog:9 > + * fast/forms/number/input-number-from-renderer-expected.txt: Added. > + * fast/forms/number/input-number-from-renderer.html: Added. Would you merge it to input-number-commit-valid-only.html please?

yosin

Comment 3 2012-06-03 22:30:36 PDT

>Did you confirm we had no other oversights? Yes. Here is call sites of parseToDoubleForNumberType other than assignment: Source/WebCore/html/HTMLMeterElement.cpp:84: return parseToDoubleForNumberType(getAttribute(minAttr), 0); Source/WebCore/html/HTMLMeterElement.cpp:98: return std::max(parseToDoubleForNumberType(getAttribute(maxAttr), std::max(1.0, min())), min()); Source/WebCore/html/parser/HTMLParserIdioms.h:47:double parseToDoubleForNumberType(const String&); Source/WebCore/html/parser/HTMLParserIdioms.h:48:double parseToDoubleForNumberType(const String&, double fallbackValue); Source/WebCore/html/parser/HTMLParserIdioms.h:49:double parseToDoubleForNumberTypeWithDecimalPlaces(const String&, unsigned*); Source/WebCore/html/parser/HTMLParserIdioms.h:50:double parseToDoubleForNumberTypeWithDecimalPlaces(const String&, unsigned*, double fallbackValue); Source/WebCore/html/parser/HTMLParserIdioms.cpp:69:double parseToDoubleForNumberType(const String& string, double fallbackValue) Source/WebCore/html/parser/HTMLParserIdioms.cpp:96:double parseToDoubleForNumberType(const String& string) Source/WebCore/html/parser/HTMLParserIdioms.cpp:98: return parseToDoubleForNumberType(string, std::numeric_limits<double>::quiet_NaN()); Source/WebCore/html/parser/HTMLParserIdioms.cpp:101:double parseToDoubleForNumberTypeWithDecimalPlaces(const String& string, unsigned *decimalPlaces, double fallbackValue) Source/WebCore/html/parser/HTMLParserIdioms.cpp:174:double parseToDoubleForNumberTypeWithDecimalPlaces(const String& string, unsigned *decimalPlaces) Source/WebCore/html/parser/HTMLParserIdioms.cpp:176: return parseToDoubleForNumberTypeWithDecimalPlaces(string, decimalPlaces, std::numeric_limits<double>::quiet_NaN()); Source/WebCore/html/NumberInputType.cpp:102: return !value.isEmpty() && !isfinite(parseToDoubleForNumberType(value)); Source/WebCore/html/NumberInputType.cpp:191: return parseToDoubleForNumberType(src, defaultValue); Source/WebCore/html/NumberInputType.cpp:196: return parseToDoubleForNumberTypeWithDecimalPlaces(src, decimalPlaces, defaultValue); Source/WebCore/html/NumberInputType.cpp:230: // Note: parseToDoubleForNumberTypeWithDecimalPlaces set zero to decimalPlaces Source/WebCore/html/NumberInputType.cpp:233: parseToDoubleForNumberTypeWithDecimalPlaces(proposedValue, &decimalPlace); Source/WebCore/html/NumberInputType.cpp:255: return standardValue.isEmpty() || isfinite(parseToDoubleForNumberType(standardValue)); Source/WebCore/html/NumberInputType.cpp:262: return isfinite(parseToDoubleForNumberType(proposedValue)) ? proposedValue : emptyString(); Source/WebCore/html/RangeInputType.cpp:224: return parseToDoubleForNumberType(src, defaultValue);

yosin

Comment 4 2012-06-03 22:36:24 PDT

Created attachment 145517 [details] Patch 2

yosin

Comment 5 2012-06-03 22:37:51 PDT

Comment on attachment 145517 [details] Patch 2 * Merge test cases into input-number-commit-valid-only.html * grep parseToDoubleForNumberType for finding other misused return value. Could you review again? Thanks in advance.

Kent Tamura

Comment 6 2012-06-03 22:43:25 PDT

Comment on attachment 145517 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=145517&action=review > LayoutTests/fast/forms/number/input-number-commit-valid-only-expected.txt:8 > +PASS "noRange enter:512"; input.value is "512" Putting a test description as a JavaScript string looks really strange... Don't hesitate to debug().

yosin

Comment 7 2012-06-03 22:49:51 PDT

Created attachment 145518 [details] Patch 3

yosin

Comment 8 2012-06-03 22:52:59 PDT

Created attachment 145519 [details] Patch 4

yosin

Comment 9 2012-06-03 22:53:27 PDT

Comment on attachment 145519 [details] Patch 4 * use function debug() in test

WebKit Review Bot

Comment 10 2012-06-03 23:49:49 PDT

Comment on attachment 145519 [details] Patch 4 Rejecting attachment 145519 [details] from commit-queue. New failing tests: http/tests/media/media-source/video-media-source-event-attributes.html Full output: http://queues.webkit.org/results/12900132

WebKit Review Bot

Comment 11 2012-06-03 23:49:53 PDT

Created attachment 145522 [details] Archive of layout-test-results from ec2-cq-02 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: ec2-cq-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

yosin

Comment 12 2012-06-04 00:02:09 PDT

Comment on attachment 145519 [details] Patch 4 Test failure on video-media-source-event-attribute.html doesn't relate to this patch.

WebKit Review Bot

Comment 13 2012-06-04 02:49:18 PDT

Comment on attachment 145519 [details] Patch 4 Clearing flags on attachment: 145519 Committed r119379: <http://trac.webkit.org/changeset/119379>

WebKit Review Bot

Comment 14 2012-06-04 02:49:23 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.