[webkit-reviews] review denied: [Bug 11274] Implement getIntersectionList(), getEnclosureList(), checkIntersection() and checkEnclosure() in SVGSVGElement : [Attachment 98655] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Rob Buis <rwlbuis at gmail.com>'s
request for review:
Bug 11274: Implement getIntersectionList(), getEnclosureList(),
checkIntersection() and checkEnclosure() in SVGSVGElement
https://bugs.webkit.org/show_bug.cgi?id=11274

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

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98655&action=review

Looks good. Still a question and a comment.

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:490
> +    if (r.isEmpty() && other.isEmpty())
> +	   return false;

Just checked isEmpty()

bool isEmpty() const { return m_width <= 0 || m_height <= 0; }

What do other browsers if intersection rect has negative width or height? Is it
maybe better to omit this return?

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:501
> +// One of the element types that can cause graphics to be drawn onto the
target canvas. Specifically: ‘circle’, ‘ellipse’,
> +// ‘image’, ‘line’, ‘path’, ‘polygon’, ‘polyline’,
‘rect’, ‘text’ and ‘use’.

Encoding problem