Bug 95705 – CSS3 calc: expressions with 'em' units do not zoom correctly.

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Mike Lawther]

As reported in http://crbug.com/145082. See attached file for repro.

[Mike Lawther]

Created attachment 161961 [details]
repro

[Mike Lawther]

Created attachment 161963 [details]
Patch

[Mike Lawther]

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 :(

[Ojan Vafai]

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.

[WebKit Review Bot]

Comment on attachment 161963 [details]
Patch

Clearing flags on attachment: 161963

Committed r127557: <http://trac.webkit.org/changeset/127557>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

(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>.

(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.

(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>.

(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.

[Mike Lawther]

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.

(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.

[Mike Lawther]

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.

[Mike Lawther]

Test rewritten at https://bugs.webkit.org/show_bug.cgi?id=95922