1178159 - Restrict stroke-linecap value to "butt" for svg circles and ellipses

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Tom Klein]

Attachment: circle and ellipse square linecaps

Other browsers ignore a stroke-linecap:"square" style attribute on svg circles and ellipses.  Should firefox do the same?

Current behavior: square linecaps on circles and ellipses are visible.

Expected behavior: circles and ellipses should never have visible linecaps.

Comment 1

[Tom Klein]

Feel free to take this, but if you'd like to give me some hints I'd be happy to work on it.

I'm not too familiar with the style code, but it seems like style rules are already read-only by the time all of the style sources have been read in, so I'm not sure where the right place to catch this is going to be.

Comment 2

[Robert Longson [:longsonr]]

You need to alter SVGContentUtils::GetStrokeOptions not to return anything other than CapStyle::BUTT for those shapes.

For extra bonus points...

If you create some public static method in SVGContentUtils that returns true for an ellipse or circle element you could also call it in nsSVGUtils::PathExtentsToMaxStrokeExtents.

nsSVGPathGeometryFrame::DidSetStyleContext would not need to clear the cached path for those shapes if they change style.

Comment 3

[Tom Klein]

Attachment: 1178159.diff

Thanks for the hints Robert.

I was hoping it would be possible to just reset stroke-linecap in the styleContext (or something along those lines) once each time style got updated, instead of having to do a check on every render, but I guess the styling system doesn't really work that way(?).

Comment 4

[Robert Longson [:longsonr]]

Comment on attachment 8627331 [details] [diff] [review]
1178159.diff

>+
>+bool
>+SVGContentUtils::ShapeTypeHasNoCorners(nsIContent* aContent) {

make that const nsIContent* aContent (and in the .h file too obviously)

> #endif
>diff --git a/layout/reftests/svg/reftest.list b/layout/reftests/svg/reftest.list
>--- a/layout/reftests/svg/reftest.list
>+++ b/layout/reftests/svg/reftest.list
>@@ -418,8 +418,9 @@ skip-if(Android) pref(layout.css.mix-ble

I think this file is in mostly alphabetical order except for the mixed blend mode stuff at the end. Can you move the test to the proper place in the file. A follow up to move the mix blend mode stuff to a subdirectory might be nice too.

>+== stroke-linecap-circle-ellipse-01.svg stroke-linecap-circle-ellipse-01-ref.svg

r=longsonr with above nits addressed

Comment 5

[Robert Longson [:longsonr]]

(In reply to Tom Klein from comment #3)
> 
> I was hoping it would be possible to just reset stroke-linecap in the
> styleContext (or something along those lines) once each time style got
> updated, instead of having to do a check on every render, but I guess the
> styling system doesn't really work that way(?).

No it doesn't because of inheritance of styles.

Comment 6

[Tom Klein]

Attachment: patch v2

https://treeherder.mozilla.org/#/jobs?repo=try&revision=528c91e19da2

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d03ae80c32d2

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d03ae80c32d2

Comment 9

[Robert Longson [:longsonr]]

Has anything changed here? I.e. Should we raise a new bug to revert this based on http://stackoverflow.com/questions/34964130/svg-stroke-linecap-doesnt-work-for-circles