Bug 48855 – Computed style is not correct for negative values in the presence of zooming

• Component: CSS
• Status: RESOLVED FIXED

[Erik Arvidsson]

Created attachment 72702 [details]
Increasing the zoom screws up the negative computed margin style

The code that adjusts values for absolute zoom does not correctly handle negatie values.

In RenderStyle.h

inline int adjustForAbsoluteZoom(int value, const RenderStyle* style)
{
    double zoomFactor = style->effectiveZoom();
    if (zoomFactor == 1)
        return value;
    // Needed because computeLengthInt truncates (rather than rounds) when scaling up.
    if (zoomFactor > 1)
        value++;

    return roundForImpreciseConversion<int, INT_MAX, INT_MIN>(value / zoomFactor);
}

The line value++ screws up negative values which ends up being incorrect.

[Erik Arvidsson]

Created attachment 72772 [details]
Patch

[Tony Chang]

Comment on attachment 72772 [details]
Patch

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

> WebCore/rendering/style/RenderStyle.h:1314
> +    if (value < 0)
> +        value--;

Is this supposed to apply both when zoomed in and zoomed out?  Does the test try both?

[Erik Arvidsson]

(In reply to comment #2)
> (From update of attachment 72772 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=72772&action=review
> 
> > WebCore/rendering/style/RenderStyle.h:1314
> > +    if (value < 0)
> > +        value--;
> 
> Is this supposed to apply both when zoomed in and zoomed out?  Does the test try both?

I manually tried both. Let me make sure that we check both in the test.

[Erik Arvidsson]

Created attachment 72850 [details]
Patch

[Darin Adler]

Comment on attachment 72850 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        Updated the test to include some negative values. I also added tests for zoom less than which in turn
> +        forced me to increase the values because we lose a lot of precision when zooming out.

"Tests for zoom less than"? Not sure what that means.

[Erik Arvidsson]

Created attachment 72857 [details]
Patch

[Erik Arvidsson]

Fixed description

[Erik Arvidsson]

Created attachment 72874 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 72874 [details]
Patch for landing

Clearing flags on attachment: 72874

Committed r71316: <http://trac.webkit.org/changeset/71316>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.