getComputedStyle for border-bottom, border-top, border-left, border-right is not implemented.

[Alexis Menard (darktears)]

getComputedStyle for border-bottom, border-top, border-left, border-right is not implemented.

[Alexis Menard (darktears)]

Created attachment 119662 [details]
Patch

[Tony Chang]

Comment on attachment 119662 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2155
> +            const int properties[3] = { CSSPropertyBorderBottomWidth, CSSPropertyBorderBottomStyle,
> +                                        CSSPropertyBorderBottomColor};

Nit: Space before } and the 4 other cases below.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2156
> +            return getCSSPropertyValuesForShorthandProperties(properties, 3, style.get(), cssValuePool);

I wonder if wtf/dtoa/utils.h should expose ARRAY_SIZE.  Would be a separate patch.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2407
> +        if (!isPropertyImplicit(properties[i])) {

What does this check do?

> LayoutTests/fast/css/getComputedStyle/getComputedStyle-border-shorthand.html:31
> +e.style.borderBottom="20em solid blue";

Can we test the different properties using a loop instead of copy/pasting all the test cases?

[Tony Chang]

Comment on attachment 119662 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2408
> +            RefPtr<CSSValue> value = getPropertyCSSValue(properties[i]);

It looks like this causes us to update layout for each property.  Maybe it would be better to pass DoNotUpdateLayout explicitly?  Alternately, maybe move the switch statement out into a new function so you also don't have to re-calculate |style|.

[Darin Adler]

Comment on attachment 119662 [details]
Patch

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

>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2156
>> +            return getCSSPropertyValuesForShorthandProperties(properties, 3, style.get(), cssValuePool);
> 
> I wonder if wtf/dtoa/utils.h should expose ARRAY_SIZE.  Would be a separate patch.

WTF_ARRAY_LENGTH is in wtf/StdLibExtras.h

[Alexis Menard (darktears)]

Created attachment 120654 [details]
Patch

[Tony Chang]

Comment on attachment 120654 [details]
Patch

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

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2144
> +            const int properties[3] = { CSSPropertyBorderBottomWidth, CSSPropertyBorderBottomStyle,
> +                                        CSSPropertyBorderBottomColor};

Nit: Space before }

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2151
> +            const int properties[3] = { CSSPropertyBorderLeftWidth, CSSPropertyBorderLeftStyle,
> +                                        CSSPropertyBorderLeftColor};

Nit: Space before }

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2159
> +            const int properties[3] = { CSSPropertyBorderRightWidth, CSSPropertyBorderRightStyle,
> +                                        CSSPropertyBorderRightColor};

Nit: Space before }

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2166
> +            const int properties[3] = { CSSPropertyBorderTopWidth, CSSPropertyBorderTopStyle,
> +                                        CSSPropertyBorderTopColor};

Nit: Space before }

[Alexis Menard (darktears)]

Created attachment 120683 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 120683 [details]
Patch for landing

Clearing flags on attachment: 120683

Committed r103774: <http://trac.webkit.org/changeset/103774>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.