Testcase ecma/TypeConversion/9.3.1-3.js failed Failure messages were: -"1e-" = -1 FAILED! expected: NaN It's been occurred because "e-" is regarded just as trailing junks while parsing exponential part. It should not be consider as trailing junks without decimal digits.
Created attachment 130965 [details] Patch
You're correct that this is a regression as per the spec, but the way in which you've fixed this regression is probably not the way we want to go. The code you've modified was pulled in from an upstream open source repository (http://code.google.com/p/double-conversion/), and we probably want to leave it alone as much as possible. Also, the fact that we can ignore trailing junk strings at the end of otherwise valid numbers if we so choose is a feature, not a bug. As you've already figured out, the issue is that the place that calls strtod expects parsing trailing junk strings to return NaN, but we're ignoring these trailing junk strings and just returning the valid prefix. Instead of removing the ability to ignore junk strings, as your current patch does, we need to pass the correct AllowJunkStringTag value to strtod when calling jsToNumber.
Comment on attachment 130965 [details] Patch r- based on Mark's comments.
(In reply to comment #2) > You're correct that this is a regression as per the spec, but the way in which you've fixed this regression is probably not the way we want to go. The code you've modified was pulled in from an upstream open source repository (http://code.google.com/p/double-conversion/), and we probably want to leave it alone as much as possible. Also, the fact that we can ignore trailing junk strings at the end of otherwise valid numbers if we so choose is a feature, not a bug. > > As you've already figured out, the issue is that the place that calls strtod expects parsing trailing junk strings to return NaN, but we're ignoring these trailing junk strings and just returning the valid prefix. Instead of removing the ability to ignore junk strings, as your current patch does, we need to pass the correct AllowJunkStringTag value to strtod when calling jsToNumber. I totally agree with your explanation on AllowJunkStringTag, but I was deeply wondering if 'e' or 'E' without signed decimal digits should be considered as trailing junk or not. I decided at that time it's not trailing junk but obvious parsing error. Isn't it correct that additional things, only after signed decimal digits, are regarded as junk in case of parsing exponential part?? I want you to make it sure this one more time. And I cannot find what you want me to check at (http://code.google.com/p/double-conversion/). Could you let me know more specific URL or something?
> I totally agree with your explanation on AllowJunkStringTag, but I was deeply wondering if 'e' or 'E' without signed decimal digits should be considered as trailing junk or not. I decided at that time it's not trailing junk but obvious parsing error. > Isn't it correct that additional things, only after signed decimal digits, are regarded as junk in case of parsing exponential part?? I want you to make it sure this one more time. According to the ECMA 262 spec section 9.3.1, if you have an exponential, you must have either 'e' or 'E' which must be always followed by a decimal string. You can compare our behavior with Chrome or Firefox. > And I cannot find what you want me to check at (http://code.google.com/p/double-conversion/). Could you let me know more specific URL or something? Nothing to look at there, I was just showing you the upstream project I was referencing. I actually have a patch ready to go for this which fixes a couple other things that were wrong too, so don't worry about submitting a new patch. Thanks for reporting this bug!
Created attachment 131146 [details] Patch
Comment on attachment 131146 [details] Patch Attachment 131146 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/11906927
Comment on attachment 131146 [details] Patch Attachment 131146 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11903978
Comment on attachment 131146 [details] Patch Attachment 131146 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11915880
Created attachment 131148 [details] Patch
Comment on attachment 131148 [details] Patch Attachment 131148 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11892943 New failing tests: fast/forms/number/ValidityState-typeMismatch-number.html fast/forms/range/input-valueasnumber-range.html fast/forms/number/input-valueasnumber-number.html
Created attachment 131194 [details] Patch
Comment on attachment 131194 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131194&action=review r=me > Source/JavaScriptCore/wtf/dtoa/double-conversion.cc:437 > // Returns true if a nonspace found and false if the end has reached. Please update this comment before committing -- and possibly the function name. "Whitespace" would be a better term than "space".
Fixed in http://trac.webkit.org/changeset/110657 with build fixes in http://trac.webkit.org/changeset/110659 and http://trac.webkit.org/changeset/110660