Bug 28934

Created attachment 38969 [details] Proposed patch stringToFloat() is in HTMLInputElement.cpp because it will be used to parse min= max= step= attributes.

Comment on attachment 38969 [details] Proposed patch It seems stringToFloat should have HTML5 in teh name. It's not a generic function, but rather a specific html5-compatible implementation. html5CompatStringToFloat html5SpecStingToFloat ? This would be better as a single if unless there is a grand plan to add a bunch more types... + switch (input->inputType()) { + case HTMLInputElement::NUMBER: + return !HTMLInputElement::stringToFloat(value, 0); + default: + return false; + } if (input->type() != HTMLInputElement::NUMBER) return false; // Check to make sure the number is valid, ignore the parsed number. return !HTMLInputElement::stringToFloat(value, 0); Otherwise looks fine.

Created attachment 38976 [details] Proposed patch (rev.2) - Followed Eric's comments

I disagree with Eric. Putting "HTML5" in function names is completely inappropriate given that this functionality will also be required for HTML 6, 7, ... and in fact HTML is switching to be unversioned (see e.g. "DOCTYPE HTML" and some of Hixie's comments). These functions should be called things like "userInputToDouble" or "inputStringToDouble" or something to indicate that the difference between them and a generic conversion to double is the fact that the origin is some kind of user input which needs additional validation or conversion. Also, nitpick: "numeber" -> "number" in at least one place.

(In reply to comment #4) > These functions should be called things like > "userInputToDouble" or "inputStringToDouble" or something to indicate that the > difference between them and a generic conversion to double is the fact that the > origin is some kind of user input which needs additional validation or > conversion. That sounds right to me too.

Putting "html5" in method or file names will just mean that a few years from now someone will have to go and rename the methods or files, just like we did recently with html4.css.

Created attachment 39031 [details] Proposed patch (rev.3) Thank you for the comments. - toDoubleForHtml5() -> formStringToDouble() I didn't use "input" "user" because this method will be used for min= max= step= attributes. - Fix a typo

Created attachment 39046 [details] Proposed patch (rev.4) - Resolve conflict with type=color.

Comment on attachment 39046 [details] Proposed patch (rev.4) LGTM.

Comment on attachment 39046 [details] Proposed patch (rev.4) Rejecting patch 39046 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1

Comment on attachment 39046 [details] Proposed patch (rev.4) Bitten by bug 29037.

Comment on attachment 39046 [details] Proposed patch (rev.4) Clearing flags on attachment: 39046 Committed r48163: <http://trac.webkit.org/changeset/48163>

All reviewed patches have been landed. Closing bug.