[webkit-reviews] review denied: [Bug 74635] getComputedStyle for border-width is not implemented. : [Attachment 119493] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Alexis Menard
<alexis.menard at openbossa.org>'s request for review:
Bug 74635: getComputedStyle for border-width is not implemented.
https://bugs.webkit.org/show_bug.cgi?id=74635

Attachment 119493: Patch
https://bugs.webkit.org/attachment.cgi?id=119493&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=119493&action=review


> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2163
> +	       if (style->borderTopStyle() == BNONE || style->borderTopStyle()
== BHIDDEN)
> +		   return cssValuePool->createValue(0,
CSSPrimitiveValue::CSS_PX);

Why does this only check the top?

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:2169
> +	       list->append(zoomAdjustedPixelValue(style->borderTop().width(),
style.get(), cssValuePool));
> +	      
list->append(zoomAdjustedPixelValue(style->borderRight().width(), style.get(),
cssValuePool));
> +	      
list->append(zoomAdjustedPixelValue(style->borderBottom().width(), style.get(),
cssValuePool));
> +	       list->append(zoomAdjustedPixelValue(style->borderLeft().width(),
style.get(), cssValuePool));

Can you just use style->borderTopWidth() like the above code for
CSSPropertyBorderTopWidth?  I think that avoids the need to check
border*Style() above?

> LayoutTests/fast/css/getComputedStyle/getComputedStyle-border-width.html:42
> +e.style.borderWidth=""
> +e.style.borderTop = "10px solid"
> +shouldBe("computedStyle.getPropertyValue('border-width')", "'10px 3px 3px
3px'");

Shouldn't right, bottom, left all be 0px here?