Bug 95705 - CSS3 calc: expressions with 'em' units do not zoom correctly.
Summary: CSS3 calc: expressions with 'em' units do not zoom correctly.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mike Lawther
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-03 20:34 PDT by Mike Lawther
Modified: 2012-09-05 18:54 PDT (History)
7 users (show)

See Also:


Attachments
repro (906 bytes, text/html)
2012-09-03 20:34 PDT, Mike Lawther
no flags Details
Patch (5.45 KB, patch)
2012-09-03 21:48 PDT, Mike Lawther
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Lawther 2012-09-03 20:34:21 PDT
As reported in http://crbug.com/145082. See attached file for repro.
Comment 1 Mike Lawther 2012-09-03 20:34:53 PDT
Created attachment 161961 [details]
repro
Comment 2 Mike Lawther 2012-09-03 21:48:47 PDT
Created attachment 161963 [details]
Patch
Comment 3 Mike Lawther 2012-09-03 21:53:27 PDT
Comment on attachment 161963 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=161963&action=review

> LayoutTests/css3/calc/zoom-with-em.html:19
> +    shouldEvaluateTo(calc.offsetWidth + "", nocalc.offsetWidth + "");

Can I do something better here? This feels a wee bit dodgy :(
Comment 4 Ojan Vafai 2012-09-04 18:42:58 PDT
Comment on attachment 161963 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=161963&action=review

>> LayoutTests/css3/calc/zoom-with-em.html:19
>> +    shouldEvaluateTo(calc.offsetWidth + "", nocalc.offsetWidth + "");
> 
> Can I do something better here? This feels a wee bit dodgy :(

You could toString or String() them. But, meh, doesn't really matter.
Comment 5 WebKit Review Bot 2012-09-04 23:28:02 PDT
Comment on attachment 161963 [details]
Patch

Clearing flags on attachment: 161963

Committed r127557: <http://trac.webkit.org/changeset/127557>
Comment 6 WebKit Review Bot 2012-09-04 23:28:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 mitz 2012-09-05 00:12:08 PDT
(In reply to comment #5)
> (From update of attachment 161963 [details])
> Clearing flags on attachment: 161963
> 
> Committed r127557: <http://trac.webkit.org/changeset/127557>

The test added in this revision, css3/calc/zoom-with-em.html, has been failing on Mountain Lion: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r127558%20(589)/css3/calc/zoom-with-em-pretty-diff.html>.
Comment 8 mitz 2012-09-05 00:13:02 PDT
(In reply to comment #7)

> The test added in this revision, css3/calc/zoom-with-em.html, has been failing on Mountain Lion: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r127558%20(589)/css3/calc/zoom-with-em-pretty-diff.html>.

It is also failing on Lion.
Comment 9 mitz 2012-09-05 00:22:34 PDT
(In reply to comment #7)
> (In reply to comment #5)
> > (From update of attachment 161963 [details] [details])
> > Clearing flags on attachment: 161963
> > 
> > Committed r127557: <http://trac.webkit.org/changeset/127557>
> 
> The test added in this revision, css3/calc/zoom-with-em.html, has been failing on Mountain Lion: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r127558%20(589)/css3/calc/zoom-with-em-pretty-diff.html>.

Landed Mac-specific expected results in <http://trac.webkit.org/r127560>.
Comment 10 mitz 2012-09-05 00:29:25 PDT
(In reply to comment #7)
> The test added in this revision, css3/calc/zoom-with-em.html, has been failing on Mountain Lion: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK1%20(Tests)/r127558%20(589)/css3/calc/zoom-with-em-pretty-diff.html>.

It is also failing on Qt: <http://build.webkit.org/results/Qt%20Linux%20Release/r127558%20(51650)/css3/calc/zoom-with-em-pretty-diff.html>. The generic expected results appear to assume subpixel layout, meaning every port that doesn’t use subpixel layout needs platform-specific expected results checked in. I don’t know whether subpixel layout is the rule or the exception among ports. If it’s an exceptional case, then perhaps the generic results should be replaced and subpixel-layout-based results should be placed in the appropriate ports’ platform directories.
Comment 11 Mike Lawther 2012-09-05 00:45:20 PDT
Oops. Thanks for checking in the rebaselines Dan!

Ideally I'll rewrite the test so it no longer reports the actual pixel values - I only care that they are equal, not what they are.
Comment 12 mitz 2012-09-05 00:50:23 PDT
(In reply to comment #11)
> Oops. Thanks for checking in the rebaselines Dan!
> 
> Ideally I'll rewrite the test so it no longer reports the actual pixel values - I only care that they are equal, not what they are.

r=me to do that. Or to change the test to use a few zoom factors that yield integer pixel amounts.
Comment 13 Mike Lawther 2012-09-05 01:07:37 PDT
I updated the expectations in http://trac.webkit.org/changeset/127563  - moving the results you kindly landed for Mac into the generic location, and moved the results I generated with the chromium port into the chromium directory.

Hopefully this should fix the bots, and I'll rewrite the test in a bit.
Comment 14 Mike Lawther 2012-09-05 18:54:34 PDT
Test rewritten at https://bugs.webkit.org/show_bug.cgi?id=95922