Bug 20674 - Very large and small numbers fail to round-trip through CSS
: Very large and small numbers fail to round-trip through CSS
Status: RESOLVED FIXED
: WebKit
CSS
: 528+ (Nightly build)
: Macintosh Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2008-09-05 14:54 PST by
Modified: 2010-10-18 14:48 PST (History)


Attachments
Testcase (1.14 KB, text/html)
2008-09-05 14:55 PST, Simon Fraser (smfr)
no flags Details
smfr's layout test with a failing baseline (2.72 KB, patch)
2009-10-06 19:33 PST, Evan Martin
no flags Review Patch | Details | Formatted Diff | Diff
smfr's layout test with a failing baseline (2.73 KB, patch)
2009-10-06 19:34 PST, Evan Martin
no flags Review Patch | Details | Formatted Diff | Diff
smfr's layout test with a failing baseline (2.79 KB, patch)
2009-10-07 10:49 PST, Evan Martin
no flags Review Patch | Details | Formatted Diff | Diff
Patch (20.15 KB, patch)
2010-10-16 13:44 PST, Simon Fraser (smfr)
zimmermann: review+
Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2008-09-05 14:54:38 PST
When converting very large or small numbers back to strings, exponential notation is used, so a value like 90010000px is returned from getComputedStyle() as 9.001e+07px.

9.001e+07px does not parse, so such a value cannot be round-tripped.

This is a particular problem with tranformation matrices, which can often contain very large or small numbers.
------- Comment #1 From 2008-09-05 14:55:15 PST -------
Created an attachment (id=23202) [details]
Testcase
------- Comment #2 From 2008-09-08 14:21:26 PST -------
See thread starting here:
<http://lists.w3.org/Archives/Public/www-style/2008Sep/0049.html>
------- Comment #3 From 2009-01-16 10:50:36 PST -------
Parsing of pixel values like "-131.33376515546504px" seems broken too.
------- Comment #4 From 2009-10-06 17:51:31 PST -------
In fixing bug 18994 (https://bugs.webkit.org/show_bug.cgi?id=18994) I'm looking at reimplementing the way we generate floats in CSS.  My fix for that bug would fix this as well.
------- Comment #5 From 2009-10-06 17:52:20 PST -------
Can you check in your test?  Or would you mind if I checked it in with appropriate baselines?
------- Comment #6 From 2009-10-06 19:23:26 PST -------
Please go ahead and check it in. We should also add some tests which read/write style.webkitTransform, since this often contains floating point numbers with lots of digits.
------- Comment #7 From 2009-10-06 19:33:10 PST -------
Created an attachment (id=40757) [details]
smfr's layout test with a failing baseline
------- Comment #8 From 2009-10-06 19:34:02 PST -------
Created an attachment (id=40758) [details]
smfr's layout test with a failing baseline
------- Comment #9 From 2009-10-06 20:47:16 PST -------
(From update of attachment 40758 [details])
Rejecting patch 40758 from commit-queue.

Failed to run "['git', 'svn', 'dcommit']" exit_code: 1
Last 500 characters of output:
ayoutTests/fast/css/large-number-round-trip.html
A repository hook failed: MERGE request failed on '/repository/webkit/trunk': Commit blocked by pre-commit hook (exit code 1) with output:
svnlook: Can't write to stream: Broken pipe

    The following files contain tab characters:

        trunk/LayoutTests/ChangeLog

    Please use spaces instead to indent.
    If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.
 at /usr/local/libexec/git-core//git-svn line 469
------- Comment #10 From 2009-10-07 10:49:04 PST -------
Created an attachment (id=40804) [details]
smfr's layout test with a failing baseline
------- Comment #11 From 2009-10-07 10:49:26 PST -------
(From update of attachment 40804 [details])
now without tabs.

FFFFFUUUUUUUUU
------- Comment #12 From 2009-10-07 11:10:33 PST -------
(From update of attachment 40804 [details])
Clearing flags on attachment: 40804

Committed r49254: <http://trac.webkit.org/changeset/49254>
------- Comment #13 From 2009-10-07 11:10:38 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #14 From 2009-10-07 11:14:02 PST -------
Patch was only a test; not fixed yet.
------- Comment #15 From 2009-10-14 14:55:16 PST -------
fixed in
http://trac.webkit.org/changeset/49585
darin++
------- Comment #16 From 2009-12-13 16:10:50 PST -------
Reopening; the change was reverted. See bug 18994.
------- Comment #17 From 2010-10-16 13:41:01 PST -------
I plan to fix CSSPrimitiveValue.cssText() via bug 20674.
------- Comment #18 From 2010-10-16 13:44:14 PST -------
Created an attachment (id=70962) [details]
Patch
------- Comment #19 From 2010-10-17 02:21:58 PST -------
(From update of attachment 70962 [details])
This is correct, and I'm tempted to r+. Though I was planning to solve this in a more generic fashion, by moving this code (formatNumber) into WTFString, as default String::number() implementation for double.
We can still move this to String::number() though as a later point, it's good to see less String::format() usage anyway.
If you prefer to land it as is, I'll r+ it later - waiting for your comment first.
------- Comment #20 From 2010-10-17 09:42:15 PST -------
I'd like to get this in, and we can do string cleanup later.
------- Comment #21 From 2010-10-17 12:39:37 PST -------
(From update of attachment 70962 [details])
Looks great, r=me. I'll take your formatNumber code as starting point for rewriting String::number(double).
------- Comment #22 From 2010-10-17 13:06:48 PST -------
http://trac.webkit.org/changeset/69928
------- Comment #23 From 2010-10-18 14:48:38 PST -------
There's a possible buffer overflow here: bug 47851.