Summary: | Very large and small numbers fail to round-trip through CSS | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
Component: | CSS | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | barraclough, cmarrin, commit-queue, darin, dino, evan, hyatt, krit, mitz, simon.fraser, zimmermann | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Mac | ||||||||||||||
OS: | OS X 10.5 | ||||||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2008-09-05 14:54:38 PDT
Created attachment 23202 [details]
Testcase
See thread starting here: <http://lists.w3.org/Archives/Public/www-style/2008Sep/0049.html> Parsing of pixel values like "-131.33376515546504px" seems broken too. 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. Can you check in your test? Or would you mind if I checked it in with appropriate baselines? 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. Created attachment 40757 [details]
smfr's layout test with a failing baseline
Created attachment 40758 [details]
smfr's layout test with a failing baseline
Comment on attachment 40758 [details]
smfr's layout test with a failing baseline
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
Created attachment 40804 [details]
smfr's layout test with a failing baseline
Comment on attachment 40804 [details]
smfr's layout test with a failing baseline
now without tabs.
FFFFFUUUUUUUUU
Comment on attachment 40804 [details] smfr's layout test with a failing baseline Clearing flags on attachment: 40804 Committed r49254: <http://trac.webkit.org/changeset/49254> All reviewed patches have been landed. Closing bug. Patch was only a test; not fixed yet. fixed in http://trac.webkit.org/changeset/49585 darin++ Reopening; the change was reverted. See bug 18994. I plan to fix CSSPrimitiveValue.cssText() via bug 20674. Created attachment 70962 [details]
Patch
Comment on attachment 70962 [details]
Patch
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.
I'd like to get this in, and we can do string cleanup later. Comment on attachment 70962 [details]
Patch
Looks great, r=me. I'll take your formatNumber code as starting point for rewriting String::number(double).
There's a possible buffer overflow here: bug 47851. |