[webkit-reviews] review granted: [Bug 76447] Differentiate between SVG/CSS width/height attributes/properties : [Attachment 122931] Patch v2

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has granted Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 76447: Differentiate between SVG/CSS width/height attributes/properties
https://bugs.webkit.org/show_bug.cgi?id=76447

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

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


r=me, with a few nits.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:882
> +		   resolvedWidth =
static_cast<int>(ceilf(intrinsicWidth.value() * style()->effectiveZoom()));

I would prefer the more compact constructor style cast
int(ceilf(intrinsicWidth.value() * style()->effectiveZoom())) here and
elsewhere. Not sure what our style policy is for this.

> Source/WebCore/svg/SVGSVGElement.h:79
> +    enum ConsiderCSSMode {
> +	   RespectCSSProperties,
> +	   IgnoreCSSProperties
>      };

I think it would be better to eliminate this enum...

> Source/WebCore/svg/SVGSVGElement.h:83
> +    Length intrinsicWidth(ConsiderCSSMode = RespectCSSProperties) const;
> +    Length intrinsicHeight(ConsiderCSSMode = RespectCSSProperties) const;

...and just have separate functions for the two cases. The code would read
better I think.