[webkit-reviews] review denied: [Bug 15473] SVG fails all 3 of Hixie's CSS intrinsic sizing tests : [Attachment 95361] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 15473: SVG fails all 3 of Hixie's CSS intrinsic sizing tests
https://bugs.webkit.org/show_bug.cgi?id=15473

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

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=95361&action=review

The code and tests look fine to me. I had some minor gripes, but I think it is
close to being ready :)

> Source/WebCore/ChangeLog:13
> +	   The *on-target-svg-absolute.xhtml tests look equal to WebKit/FF but
Opera fails them, likely a relict of the differnt

s/differnt/different

> Source/WebCore/rendering/svg/RenderSVGRoot.h:42
> +    FloatSize computeIntrinsicRatio(bool& isPercentageIntrinsicSize) const;

I had to look at the header to see that the bool was an in-out param. Can you
do anything to make that more clear? Now it seems like a sort of a getter that
also happens to set a passed in bool as side-effect. Maybe use Info in the name
and pass in both data members that need to be calculated?

> LayoutTests/ChangeLog:6
> +	   https://bugs.webkit.org/show_bug.cgi?id=15473

I think this needs more wording. Maybe you can repeat the paragraph about the
test in the WebCore/ChangeLog and explain a bit detailed what the tests do.