Created attachment 144834 [details] HTML file contains failing SVG rect negative rx,ry test cases The SVG specification says that negative values for rx,ry "rect" corner radii aren't allowed and that if "if neither rx nor ry are properly specified, then set both rx and ry to 0. (This will result in square corners.)" - http://www.w3.org/TR/SVG/shapes.html#RectElement. Similarly, if only one of rx,ry is negative then the other attribute (default 0) should be used for both. The current implementation fails in all three cases: <rect x="100" y="100" width="200" height="100" rx="-90" ry="-40"/> <rect x="100" y="100" width="200" height="100" rx="-90" ry="40"/> <rect x="100" y="100" width="200" height="100" rx="90" ry="-40"/>
Created attachment 144836 [details] Screenshot of the failing test cases.
Created attachment 145110 [details] Patch
I'd assigned the bug to myself because I had a fix for it. I'd put the changes in Path::addRoundedRect(), where the other checks for the corner constraints from the SVG spec are already.
Not to put too fine a point on it, but here's the updated version of addRoundedRect(). I'd merged the original comments about the SVG spec with a new one. void Path::addRoundedRect(const FloatRect& rect, const FloatSize& roundingRadii, RoundedRectStrategy strategy) { if (rect.isEmpty()) return; FloatSize radius(roundingRadii); FloatSize halfSize(rect.width() / 2, rect.height() / 2); // Apply the SVG corner radius constraints, per the rect section of the SVG shapes spec: if // one of rx,ry is negative, then the other corner radius value is used. If both values are // negative then rx = ry = 0. If rx is greater than half of the width of the rectangle // then set rx to half of the width; ry is handled similarly. if (radius.width() < 0) radius.setWidth((radius.height() < 0) ? 0 : radius.height()); if (radius.height() < 0) radius.setHeight(radius.width()); if (radius.width() > halfSize.width()) radius.setWidth(halfSize.width()); if (radius.height() > halfSize.height()) radius.setHeight(halfSize.height()); addPathForRoundedRect(rect, radius, radius, radius, radius, strategy); }
Hi, (In reply to comment #3) > I'd assigned the bug to myself because I had a fix for it. I'd put the changes in Path::addRoundedRect(), where the other checks for the corner constraints from the SVG spec are already. I am sorry, totally missed the assigned! We are still discussing on IRC what is the best place to fix this.
Created attachment 145163 [details] Patch
I've uploaded a revised patch per the krit,rbuis IRC discussion. The reftest is essentially the same as the one Rob provided, modulo a little gratuitous reformatting.
Comment on attachment 145163 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145163&action=review Looks good to me. Just snippets to get r+. > LayoutTests/svg/custom/rect-negative-corner-radii-expected.svg:2 > +<svg xmlns:svg="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg" version="1.0" width="500" height="500"> xmlns:svg="http://www.w3.org/2000/svg" Is unnecessary. > LayoutTests/svg/custom/rect-negative-corner-radii.svg:2 > +<svg xmlns:svg="http://www.w3.org/2000/svg" xmlns="http://www.w3.org/2000/svg" version="1.0" width="500" height="500"> Ditto.
Created attachment 145170 [details] Patch
Removed redundant namespace attributes from the reftest.
Comment on attachment 145170 [details] Patch Clearing flags on attachment: 145170 Committed r119182: <http://trac.webkit.org/changeset/119182>
All reviewed patches have been landed. Closing bug.