[Webkit-unassigned] [Bug 93290] getBBox() returns (0, 0) when width or height is zero.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=93290





--- Comment #9 from Philip Rogers <pdr at google.com>  2012-08-14 21:35:22 PST ---
(From update of attachment 158473)
View in context: https://bugs.webkit.org/attachment.cgi?id=158473&action=review

I'm not a reviewer but I know this code fairly well. Overall this is a good start! I left a note below about the bug that led to your test failures.

There is also a subtle bug if you have a "fallback" case like:
<rect vector-effect="non-scaling-stroke" height="100" width="0"/>
Essentially what is happening is we "fall back" to path rendering code for shapes with non-scaling-strokes. To fix this you can make a very similar change in SVGPathData::updatePathFromRectElement() (and similarly for circle and ellipse).

> Source/WebCore/rendering/svg/RenderSVGEllipse.cpp:69
> +    if (m_radii.isZero() || m_radii.width() < 0 || m_radii.height() < 0)

Can we update the comment here to say something like: Spec: A negative value is an error. A value of zero disables rendering of the element.

There is a bug here because if the width or height is zero, we will end up creating the correct fill bounding box, but then we actually inflate the stroke bounding box so that it renders. One way to fix this is to add a check if the width or height is zero before inflating the stroke bounding box.

> Source/WebCore/rendering/svg/RenderSVGRect.cpp:69
> +    if (boundingBoxSize.isZero() || boundingBoxSize.width() < 0 || boundingBoxSize.height() < 0)

Ditto about the comment and bug.

> LayoutTests/ChangeLog:8
> +        Add new test to check bounding box size which is (0, 100).

I think it will be best to add three tests here: one for <rect>, one for <circle>, and one for <ellipse>.

If you want to fix the non-scaling-stroke bug as well, you could probably test both rect and non-scaling-stroke rect in the same test.

> LayoutTests/svg/custom/getBBox-has-one-zero-value.html:3
> +<!-- Test that svg bounding boxes has a zero width or hegith -->

Can you change this to "Test that the bounding box is valid for a zero-width rect.", or something similar? It's a little more clear what exactly is being tested.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.