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

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

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.

Created attachment 145514 [details]
Patch 1

[Kent Tamura]

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?

>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);

Created attachment 145517 [details]
Patch 2

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 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().

Created attachment 145518 [details]
Patch 3

Created attachment 145519 [details]
Patch 4

Comment on attachment 145519 [details]
Patch 4

* use function debug() in test

[WebKit Review Bot]

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]

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

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 on attachment 145519 [details]
Patch 4

Clearing flags on attachment: 145519

Committed r119379: <http://trac.webkit.org/changeset/119379>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.