Closed Bug 1178159 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: twointofive, Assigned: twointofive)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
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.
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.
Attached patch 1178159.diff (obsolete) — Splinter Review
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(?).
Assignee: nobody → twointofive
Attachment #8627331 - Flags: review?(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
Attachment #8627331 - Flags: review?(longsonr) → review+
(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.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d03ae80c32d2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
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
Flags: needinfo?(twointofive)
Depends on: 1242147
Flags: needinfo?(twointofive)
You need to log in before you can comment on or make changes to this bug.