[webkit-reviews] review denied: [Bug 13709] Table border doesn't show up : [Attachment 196129] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied Suchit Agrawal
<a.suchit at samsung.com>'s request for review:
Bug 13709: Table border doesn't show up
https://bugs.webkit.org/show_bug.cgi?id=13709

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

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=196129&action=review


> Source/WebCore/css/StyleBuilder.cpp:586
> +template <> unsigned computeLength<unsigned>(RenderStyle* style,
RenderStyle* rootStyle, float zoom, CSSPrimitiveValue* primitiveValue)

So ApplyPropertyComputeLength<unsigned,... version is only used for border
properties, making this special rounding behavior border specific. This is a
very indirect and confusing way of getting border specific behavior.

I think it would be better to refactor this a bit. The current
ApplyPropertyComputeLength could be replaced with several simpler versions

ApplyPropertyComputeBorderWidth (for border, outline and similar properties.
this would have your new rounding behavior)
ApplyPropertyComputeTextSpacing (for word and letter-spacing, only case that
has svgZoomEnabled)
ApplyPropertyComputeTransformOriginZ (this is the only float case and seems to
want the rounding behavior only. maybe not even needed.)

Each of these can be much simpler and understandable than the current overly
generic type. Most of the template parameters can be removed.