Bug 50974 – getComputedStyle() returns wrong values for zoomed elements when display is none

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

[Beth Dakin]

This is a follow-on from https://bugs.webkit.org/show_bug.cgi?id=32230 getComputedStyle() returns different values for different zoom levels.

If an element (or one of the element's ancestors) has display:none, this bug still occurs - the value returned by getComputedStyle() is multiplied by the zoom factor. 

A brief summary of the issue (see the original issue for more detail):
The values returned by getComputedStyle() are scaled according to the CSS zoom property, which is different from all other DOM APIs (e.g. offsetHeight/Width) and other browsers (Gecko returns unscaled values).

The return values should be unscaled.

<rdar://problem/8522731>

Patch forthcoming!

[Beth Dakin]

Created attachment 76437 [details]
Test case

[Beth Dakin]

Created attachment 76438 [details]
Patch

[David Levin]

Comment on attachment 76438 [details]
Patch

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

> WebCore/css/CSSComputedStyleDeclaration.cpp:1550
> +

I don't know this area but I do see a pattern here.

Why not create a function like this:

PassRefPtr<CSSValue> get{FillInNameHere}(const Length& length, const RenderStyle* style)
{
    if (length.isFixed())
        return zoomAdjustedPixelValue(width.value(), style);
    return CSSPrimitiveValue::create(width);
}

and use this in all of these places
?

[Darin Adler]

Comment on attachment 76438 [details]
Patch

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

r=me, but please consider factoring out the repetitive code into a function

>> WebCore/css/CSSComputedStyleDeclaration.cpp:1550
>> +
> 
> I don't know this area but I do see a pattern here.
> 
> Why not create a function like this:
> 
> PassRefPtr<CSSValue> get{FillInNameHere}(const Length& length, const RenderStyle* style)
> {
>     if (length.isFixed())
>         return zoomAdjustedPixelValue(width.value(), style);
>     return CSSPrimitiveValue::create(width);
> }
> 
> and use this in all of these places
> ?

I was going to make the same suggestion. No need to use the word “get”, though!

[Beth Dakin]

Thanks all Fixed with r74045.